Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(accident-notification): procure for accident notifications #16337

Merged
merged 18 commits into from
Dec 12, 2024

Conversation

jonnigs
Copy link
Member

@jonnigs jonnigs commented Oct 9, 2024

What

Add in behaviour for accident notification if the user is logged in with procure

Why

So that users can submit accident notification on behalf of a company or a person

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

  • New Features

    • Introduced new sections and messages in the Accident Notification and Prerequisites forms.
    • Added new options for notification recipients.
    • Added new enum values and types to improve functionality.
    • New role PROCURER added for enhanced role management.
  • Bug Fixes

    • Corrected typos in message identifiers.
  • Refactor

    • Removed unnecessary exports and constants to streamline the codebase.
    • Updated method signatures for clarity and consistency.
    • Updated import paths for constants to improve organization.
  • Chores

    • Updated enums and types to enhance data handling across various forms.

@jonnigs jonnigs added the deploy-feature Deploys features to dev label Oct 9, 2024
Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 32.55814% with 29 lines in your changes missing coverage. Please review.

Project coverage is 35.24%. Comparing base (37fe92b) to head (e2bb3d7).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tification/src/lib/AccidentNotificationTemplate.ts 26.66% 11 Missing ⚠️
...ionForSection/whoIsTheNotificationForMultiField.ts 0.00% 6 Missing ⚠️
...mationMultiField/applicantInformationMultiField.ts 0.00% 5 Missing ⚠️
...ion/src/utils/getWhoIstheNotificationForOptions.ts 0.00% 2 Missing ⚠️
...src/forms/PrerequisitesForm/dataHandlingSection.ts 0.00% 1 Missing ⚠️
...src/forms/PrerequisitesForm/externalDataSection.ts 0.00% 1 Missing ⚠️
...-notification/src/forms/PrerequisitesForm/index.ts 0.00% 1 Missing ⚠️
...ms/PrerequisitesProcureForm/externalDataSection.ts 0.00% 1 Missing ⚠️
...cation/src/forms/PrerequisitesProcureForm/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16337      +/-   ##
==========================================
- Coverage   35.76%   35.24%   -0.53%     
==========================================
  Files        6931     6908      -23     
  Lines      147977   149605    +1628     
  Branches    42173    43148     +975     
==========================================
- Hits        52931    52730     -201     
- Misses      95046    96875    +1829     
Flag Coverage Δ
air-discount-scheme-backend 48.20% <ø> (ø)
air-discount-scheme-web 0.00% <ø> (ø)
api 3.33% <ø> (ø)
api-catalogue-services 75.81% <ø> (ø)
api-domains-air-discount-scheme 37.90% <ø> (ø)
api-domains-assets 26.71% <ø> (ø)
api-domains-auth-admin 48.49% <ø> (ø)
api-domains-communications 39.42% <ø> (ø)
api-domains-criminal-record 47.92% <ø> (ø)
api-domains-driving-license 44.81% <ø> (ø)
api-domains-education 31.22% <ø> (ø)
api-domains-health-insurance 35.33% <ø> (ø)
api-domains-mortgage-certificate 35.16% <ø> (ø)
api-domains-payment-schedule 42.04% <ø> (ø)
application-api-files 61.73% <ø> (ø)
application-core 76.01% <ø> (ø)
application-system-api 38.75% <46.66%> (-0.02%) ⬇️
application-template-api-modules 27.80% <46.66%> (+<0.01%) ⬆️
application-templates-accident-notification 28.83% <0.00%> (-0.10%) ⬇️
application-templates-car-recycling 3.12% <ø> (ø)
application-templates-criminal-record 25.78% <0.00%> (ø)
application-templates-driving-license 18.12% <0.00%> (ø)
application-templates-estate 13.79% <ø> (ø)
application-templates-example-payment 24.71% <0.00%> (ø)
application-templates-financial-aid 14.42% <ø> (ø)
application-templates-general-petition 23.13% <ø> (ø)
application-templates-inheritance-report 6.54% <ø> (ø)
application-templates-marriage-conditions 15.11% <ø> (ø)
application-templates-mortgage-certificate 43.60% <0.00%> (+0.04%) ⬆️
application-templates-parental-leave 29.95% <ø> (ø)
application-types 6.57% <ø> (ø)
application-ui-components 1.24% <ø> (ø)
application-ui-shell 22.47% <ø> (ø)
auth-admin-web 2.43% <ø> (ø)
auth-nest-tools 31.52% <ø> (ø)
auth-shared 75.00% <ø> (ø)
clients-charge-fjs-v2 28.35% <ø> (ø)
clients-driving-license 40.79% <ø> (ø)
clients-driving-license-book 43.83% <ø> (ø)
clients-financial-statements-inao 49.59% <ø> (ø)
clients-license-client 1.26% <ø> (ø)
clients-middlewares 73.21% <ø> (+0.08%) ⬆️
clients-regulations 42.87% <ø> (ø)
clients-rsk-company-registry 31.04% <ø> (ø)
clients-rsk-personal-tax-return 38.32% <ø> (ø)
clients-smartsolutions 12.77% <ø> (ø)
clients-syslumenn 49.41% <ø> (ø)
clients-zendesk 50.46% <ø> (ø)
cms 0.40% <ø> (ø)
cms-translations 38.76% <ø> (ø)
content-search-index-manager 95.65% <ø> (ø)
content-search-toolkit 8.20% <ø> (ø)
contentful-apps 4.72% <ø> (ø)
dokobit-signing 62.10% <ø> (ø)
email-service 60.07% <ø> (ø)
feature-flags 90.76% <ø> (ø)
file-storage 45.86% <ø> (ø)
financial-aid-backend 51.40% <ø> (ø)
financial-aid-shared 17.88% <ø> (ø)
icelandic-names-registry-backend 54.33% <ø> (ø)
infra-nest-server 48.06% <ø> (ø)
infra-tracing 69.63% <ø> (ø)
island-ui-core 30.92% <ø> (ø)
judicial-system-api 20.16% <ø> (ø)
judicial-system-audit-trail 68.94% <ø> (ø)
judicial-system-formatters 78.28% <ø> (ø)
judicial-system-message 66.47% <ø> (ø)
judicial-system-message-handler 48.23% <ø> (ø)
judicial-system-scheduler 70.97% <ø> (ø)
judicial-system-types 42.22% <ø> (ø)
judicial-system-web 27.89% <ø> (ø)
license-api 42.97% <ø> (-0.02%) ⬇️
localization 10.15% <ø> (ø)
logging 58.02% <ø> (ø)
message-queue 67.76% <ø> (ø)
nest-audit 65.78% <ø> (ø)
nest-aws 51.93% <ø> (ø)
nest-config 76.38% <ø> (ø)
nest-core 53.16% <ø> (ø)
nest-feature-flags 51.00% <ø> (ø)
nest-problem 45.64% <ø> (ø)
nest-sequelize 94.44% <ø> (ø)
nest-swagger 51.13% <ø> (ø)
nova-sms 61.42% <ø> (ø)
portals-admin-regulations-admin 1.80% <ø> (ø)
portals-core 19.83% <ø> (ø)
regulations 16.78% <ø> (ø)
residence-history 85.00% <ø> (ø)
services-auth-admin-api 52.55% <ø> (ø)
services-auth-delegation-api 58.53% <ø> (+0.08%) ⬆️
services-auth-ids-api 52.55% <ø> (+<0.01%) ⬆️
services-auth-public-api 49.40% <ø> (ø)
services-sessions 65.35% <ø> (ø)
services-university-gateway 49.42% <ø> (-0.09%) ⬇️
services-user-notification 46.57% <ø> (-0.01%) ⬇️
services-user-profile 56.98% <ø> (+0.08%) ⬆️
shared-components 30.06% <ø> (ø)
shared-form-fields 33.88% <ø> (ø)
shared-mocking 58.64% <ø> (ø)
shared-pii 92.85% <ø> (ø)
shared-problem 87.50% <ø> (ø)
shared-utils 27.69% <ø> (ø)
skilavottord-ws 24.31% <ø> (ø)
web 2.43% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/accident-notification/src/dataProviders/index.ts 100.00% <100.00%> (ø)
...t-notification/src/fields/DateOfAccident/index.tsx 0.00% <ø> (ø)
...ent-notification/src/fields/FormOverview/index.tsx 0.00% <ø> (ø)
...m/aboutTheAccidentSection/attachmentsSubSection.ts 0.00% <ø> (ø)
...Form/aboutTheAccidentSection/locationSubSection.ts 0.00% <ø> (ø)
...m/aboutTheAccidentSection/workMachineSubSection.ts 0.00% <ø> (ø)
...entNotificationForm/applicantInformationSection.ts 0.00% <ø> (ø)
...cation/src/forms/AccidentNotificationForm/index.ts 0.00% <ø> (ø)
...ionForSection/juridicialPersonCompanySubSection.ts 0.00% <ø> (ø)
...ationForSection/powerOfAttorneyUploadSubSection.ts 0.00% <ø> (ø)
... and 24 more

... and 255 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37fe92b...e2bb3d7. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Oct 9, 2024

Datadog Report

All test runs b30ad94 🔗

98 Total Test Services: 0 Failed, 90 Passed
🔻 Test Sessions change in coverage: 3 decreased, 1 increased (+0.04%), 195 no change

Test Services
This report shows up to 10 services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
air-discount-scheme-backend 0 0 0 63 0 30s N/A Link
air-discount-scheme-web 0 0 0 2 0 8.34s N/A Link
api 0 0 0 4 0 3.07s N/A Link
api-catalogue-services 0 0 0 23 0 11.95s N/A Link
api-domains-air-discount-scheme 0 0 0 6 0 21.13s N/A Link
api-domains-assets 0 0 0 3 0 12.35s N/A Link
api-domains-auth-admin 0 0 0 18 0 14.53s N/A Link
api-domains-communications 0 0 0 5 0 35.79s N/A Link
api-domains-criminal-record 0 0 0 5 0 10.06s N/A Link
api-domains-driving-license 0 0 0 23 0 34.06s N/A Link

🔻 Code Coverage Decreases vs Default Branch (3)

  • clients-middlewares - jest 75.6% (-0.1%) - Details
  • license-api - jest 34.09% (-0.03%) - Details
  • services-university-gateway - jest 45.25% (-0.02%) - Details

@jonnigs jonnigs changed the title feat(accident-notification): start of procure for accident notifications feat(accident-notification): procure for accident notifications Oct 10, 2024
@jonnigs jonnigs marked this pull request as ready for review October 15, 2024 13:08
@jonnigs jonnigs requested review from a team as code owners October 15, 2024 13:08
Copy link
Contributor

coderabbitai bot commented Oct 15, 2024

Walkthrough

The pull request introduces various modifications to the accident notification application templates. Key changes include the addition of a new enum value, updates to method signatures, and the removal of certain exported sections and constants. New sections and constants are introduced in the prerequisites forms, while several enums are removed across different modules. Additionally, updates to message handling and type definitions are made to reflect the changes in the application's structure.

Changes

File Path Change Summary
libs/application/templates/accident-notification/src/dataProviders/index.ts New exports added: IdentityApi, NationalRegistryUserApi from @island.is/application/types.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/applicantInformationSection.ts Removed unnecessary blank lines.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/betaTestSection.ts Export removed: betaTestSection.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/agreementDescriptionMultiField.ts Export removed: agreementDescriptionMultiField.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/index.ts Export removed: externalDataSection.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/index.ts Sections removed: betaTestSection, externalDataSection.
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/whoIsTheNotificationForMultiField.ts Import updated for WhoIsTheNotificationForEnum.
libs/application/templates/accident-notification/src/forms/PrerequsitesForm/dataHandlingSection.ts New section added: dataHandlingSection.
libs/application/templates/accident-notification/src/forms/PrerequsitesForm/externalDataSection.ts Method signature updated: accidentNotificationSubSection to externalDataSection.
libs/application/templates/accident-notification/src/forms/PrerequsitesForm/index.ts Constant added: PrerequisitesForm: Form.
libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/externalDataSection.ts New exported constant: externalDataSection.
libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/index.ts Method added: PrerequisitesProcureForm: Form.
libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts Updated enum Roles to include PROCURER. Updated method signature and state initialization.
libs/application/templates/accident-notification/src/lib/messages/applicantInformation.ts New section added: procure and corrected typo in id.
libs/application/templates/accident-notification/src/lib/messages/externalData.ts New messages added: procureDescription, subTitle in nationalRegistry.
libs/application/templates/accident-notification/src/lib/messages/whoIsTheNotificationFor.ts New messages added: procureDescription and label powerOfAttorneyProcure.
libs/application/templates/accident-notification/src/types/index.ts Removed enum DataProviderTypes and added new types.
libs/application/templates/accident-notification/src/utils/getWhoIstheNotificationForOptions.ts New constants added: whoIsTheNotificationForOptions, whoIsTheNotificationForProcureOptions.
libs/application/templates/announcement-of-death/src/types/index.ts Removed enum: DataProviderTypes.
libs/application/templates/family-matters/children-residence-change-v2/src/types/index.ts Removed enum: DataProviderTypes.
libs/application/templates/general-fishing-license/src/dataProviders/index.ts Method signature updated: externalDataId from identityRegistry to identity.
libs/application/templates/general-fishing-license/src/lib/dataSchema.ts Property name updated from identityRegistry to identity.
libs/application/templates/general-fishing-license/src/types/index.ts Removed enum: DataProviderTypes.
libs/application/templates/public-debt-payment-plan/src/fields/Overview/Overview.tsx No alterations to exported entities.
libs/application/ui-forms/src/lib/applicantInformationMultiField/applicantInformationMultiField.ts No alterations to exported entities.
libs/application/ui-forms/src/lib/applicantInformationMultiField/types.ts Interface updated: identityRegistry to identity.

Possibly related PRs

Suggested labels

automerge

Suggested reviewers

  • albinagu

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (25)
libs/application/templates/accident-notification/src/forms/PrerequsitesForm/index.ts (2)

8-15: LGTM: Form structure is well-defined and supports reusability.

The PrerequisitesForm is well-structured using the buildForm function, which promotes consistency across the application. The form includes necessary properties and incorporates child sections, supporting modularity and reusability.

Consider adding an explicit type annotation to PrerequisitesForm for improved clarity:

export const PrerequisitesForm: Form = buildForm({
  // ... existing properties
})

1-15: Adherence to coding guidelines confirmed with a minor suggestion.

The code adheres well to the guidelines for files in the libs/**/* pattern:

  1. It exports a reusable form component.
  2. TypeScript is used effectively for type definitions.
  3. The modular structure supports tree-shaking and efficient bundling.

To further improve type exports, consider adding a named export for the PrerequisitesForm type:

import { Form } from '@island.is/application/types'

export const PrerequisitesForm: Form = buildForm({
  // ... existing properties
})

export type { Form as PrerequisitesFormType }

This allows consumers to import both the form and its type if needed.

libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/index.ts (2)

1-6: LGTM! Consider grouping imports for better readability.

The imports are appropriate and well-organized. They include necessary dependencies from external packages and local modules.

For improved readability, consider grouping the imports as follows:

  1. External package imports
  2. Local type/interface imports
  3. Local component/asset imports
  4. Local utility/helper imports
import { buildForm } from '@island.is/application/core'
import { Form } from '@island.is/application/types'

import { application } from '../../lib/messages'
import Logo from '../../assets/Logo'

import { dataHandlingSection } from '../PrerequsitesForm/dataHandlingSection'
import { externalDataSection } from './externalDataSection'

8-15: LGTM! Consider using a type assertion for improved type safety.

The form definition is well-structured and includes all necessary components for a prerequisites form. The reuse of dataHandlingSection from another form promotes code reusability, which aligns with the coding guidelines for this directory.

For improved type safety, consider using a type assertion for the form configuration object:

export const PrerequisitesProcureForm: Form = buildForm({
  id: 'PrerequisitesProcureForm',
  title: application.general.name,
  logo: Logo,
  renderLastScreenButton: true,
  renderLastScreenBackButton: true,
  children: [dataHandlingSection, externalDataSection],
} as const)

This ensures that the configuration object is treated as a readonly object, preventing accidental modifications.

libs/application/ui-forms/src/lib/applicantInformationMultiField/types.ts (1)

Incomplete Refactoring Detected

The renaming from identityRegistry to identity was not fully applied across the codebase. The file libs/application/templates/public-debt-payment-plan/src/lib/messages/externalData.ts still contains references to identityRegistry.

  • Affected File:
    • libs/application/templates/public-debt-payment-plan/src/lib/messages/externalData.ts

Please update this file to replace all instances of identityRegistry with identity to ensure consistency and prevent potential issues.

🔗 Analysis chain

Line range hint 5-15: Rename of identityRegistry to identity looks good, but verify its impact.

The change from identityRegistry to identity in the ApplicantInformationInterface is straightforward and maintains the existing structure. This change adheres to the coding guidelines for the libs directory:

  1. It preserves the reusability of the interface across different NextJS apps.
  2. It continues to use TypeScript effectively for defining the interface structure.
  3. The change doesn't affect tree-shaking or bundling practices.

To ensure this change doesn't introduce any issues in the codebase, please run the following script:

This script will help identify any places where the old identityRegistry might still be in use and confirm that the new identity property is being used correctly throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of 'identityRegistry' and verify 'identity' usage

# Search for any remaining 'identityRegistry' references
echo "Searching for remaining 'identityRegistry' references:"
rg --type typescript --type javascript 'identityRegistry'

# Verify correct usage of 'identity'
echo "Verifying correct usage of 'identity':"
rg --type typescript --type javascript 'identity.*data.*name'

Length of output: 409


Script:

#!/bin/bash
# Description: Check for any remaining usage of 'identityRegistry' and verify 'identity' usage

# Search for any remaining 'identityRegistry' references
echo "Searching for remaining 'identityRegistry' references:"
rg 'identityRegistry' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'

# Verify correct usage of 'identity'
echo "Verifying correct usage of 'identity':"
rg 'identity.*data.*name' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'

Length of output: 3654


Script:

#!/bin/bash
# Description: List all files containing 'identityRegistry' to facilitate manual review and refactoring

echo "Listing all files with 'identityRegistry' references:"
rg 'identityRegistry' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' --files-with-matches

Length of output: 292

libs/application/templates/accident-notification/src/utils/getWhoIstheNotificationForOptions.ts (2)

4-21: LGTM: Well-structured constant with a minor suggestion for improvement.

The whoIsTheNotificationForOptions constant is well-designed, promoting reusability and consistency. It effectively maps enum values to human-readable labels, which is a good practice.

To enhance type safety, consider adding an explicit type annotation:

export const whoIsTheNotificationForOptions: Array<{value: WhoIsTheNotificationForEnum, label: string}> = [
  // ... existing array content
]

This will ensure that all array elements conform to the expected structure.


23-32: LGTM: Well-structured constant with suggestions for improvement.

The whoIsTheNotificationForProcureOptions constant is well-designed and consistent with the previous constant. It appropriately tailors options for procurement scenarios.

Suggestions for improvement:

  1. Add an explicit type annotation for enhanced type safety:
export const whoIsTheNotificationForProcureOptions: Array<{value: WhoIsTheNotificationForEnum, label: string}> = [
  // ... existing array content
]
  1. Consider creating a shared type for both constants to ensure consistency:
type NotificationOption = {
  value: WhoIsTheNotificationForEnum;
  label: string;
};

export const whoIsTheNotificationForOptions: NotificationOption[] = [
  // ... existing array content
];

export const whoIsTheNotificationForProcureOptions: NotificationOption[] = [
  // ... existing array content
];

These changes will improve type safety and maintain consistency between the two constants.

libs/application/templates/accident-notification/src/forms/PrerequsitesForm/dataHandlingSection.ts (1)

23-36: LGTM: Extra information field is well-structured with a minor suggestion.

The custom field for extra information is properly configured using the buildCustomField function. The use of the 'DescriptionWithLink' component enhances user experience and reusability. Sourcing all text content from externalData supports good internationalization practices.

Consider using TypeScript's as const assertion for the configuration object to enhance type safety:

{
  descriptionFirstPart: externalData.extraInformation.description,
  descriptionSecondPart: '',
  linkName: externalData.extraInformation.linkText,
  url: externalData.extraInformation.link,
} as const

This will ensure that the properties are treated as read-only literal types, providing stronger type checking.

libs/application/templates/accident-notification/src/lib/messages/applicantInformation.ts (2)

58-69: Review the new procure section

The new procure section has been added successfully, following the structure of other sections in the file. However, there are a couple of points to address:

  1. The defaultMessage for titill seems incomplete. It currently ends with "um ", which looks like it's missing some information.
  2. The description for both titill and name messages is identical. Consider making them more specific to accurately describe each message's purpose.

Consider updating the procure section as follows:

 procure: defineMessages({
   titill: {
     id: 'an.application:applicantInfo.procure.title',
-    defaultMessage: 'Upplýsingar um ',
+    defaultMessage: 'Upplýsingar um umboðsaðila', // or appropriate complete message
-    description: 'Name of the procure identity',
+    description: 'Title for the procure identity section',
   },
   name: {
     id: 'an.application:applicantInfo.procure.name',
     defaultMessage: 'Nafn',
-    description: 'Name of the procure identity',
+    description: 'Label for the name field of the procure identity',
   },
 }),

Line range hint 46-46: Fix typo in postalCode label id

There's a typo in the id for the postalCode label. The word "labels" is misspelled as "abels".

Please apply the following fix:

-      id: 'an.application:applicantInfo.abels.postalCode',
+      id: 'an.application:applicantInfo.labels.postalCode',
libs/application/templates/accident-notification/src/lib/messages/whoIsTheNotificationFor.ts (2)

20-24: LGTM! Consider adding a translation note.

The addition of procureDescription aligns well with the PR objectives and follows the existing structure. It provides clear information about submitting notifications on behalf of others.

Consider adding a translation note to help translators understand the context:

 procureDescription: {
   id: 'an.application:whoIsTheNotificationFor.procureDescription',
   defaultMessage: `Í umboði er hægt að tilkynna slys fyrir einstaklinga sem þú ert með umboð frá eða fyrir starfsmann lögaðila. Foreldrar og forráðamenn geta líka sent inn tilkynningu fyrir hönd barna sem þeir fara með forsjá yfir. Stofnanir, samtök og félög sem eru virk á sviði persónuverndar geta sent inn tilkynningu án umboðs að uppfylltum skilyrðum 80. gr. reglugerðar (ESB) 2016/679 (almennu persónuverndarreglugerðarinnar).`,
   description: 'Description for who is the notifaction for',
+  note: 'This message explains the conditions for submitting notifications on behalf of others using power of attorney.',
 },

42-46: LGTM! Consider updating the description.

The addition of powerOfAttorneyProcure is well-structured and aligns with the PR objectives. However, the description could be more specific to differentiate it from the existing powerOfAttorney label.

Consider updating the description to be more specific:

 powerOfAttorneyProcure: {
   id: 'an.application:whoIsTheNotificationFor.labels.powerOfAttorneyProcure',
   defaultMessage: 'Einstakling',
-  description: 'Label for power of attorney option',
+  description: 'Label for power of attorney option specifically for individuals',
 },
libs/application/ui-forms/src/lib/applicantInformationMultiField/applicantInformationMultiField.ts (6)

38-38: LGTM with a minor suggestion for consistency

The update from identityRegistry to identity is correct and maintains the existing functionality. The use of optional chaining and fallback to an empty string ensures type safety and a valid default value.

For consistency with the other fields, consider moving the application.externalData?.nationalRegistry?.data?.fullName check to the end of the chain:

defaultValue: (application: ApplicantInformationInterface) =>
  application.externalData?.identity?.data?.name ??
  application.externalData?.nationalRegistry?.data?.fullName ??
  '',

This change would prioritize the new identity data source while still maintaining the fallback to nationalRegistry.


50-50: LGTM with a minor suggestion for consistency

The update from identityRegistry to identity is correct and maintains the existing functionality. The use of optional chaining and fallback to an empty string ensures type safety and a valid default value.

For consistency with the other fields and to prioritize the new identity data source, consider reordering the checks:

defaultValue: (application: ApplicantInformationInterface) =>
  application.externalData?.identity?.data?.nationalId ??
  application.externalData?.nationalRegistry?.data?.nationalId ??
  '',

This change would prioritize the new identity data source while still maintaining the fallback to nationalRegistry.


62-62: LGTM with a minor suggestion for consistency

The update from identityRegistry to identity is correct and maintains the existing functionality. The use of optional chaining and fallback to an empty string ensures type safety and a valid default value.

For consistency with the other fields and to prioritize the new identity data source, consider reordering the checks:

defaultValue: (application: ApplicantInformationInterface) =>
  application.externalData?.identity?.data?.address?.streetAddress ??
  application.externalData?.nationalRegistry?.data?.address?.streetAddress ??
  '',

This change would prioritize the new identity data source while still maintaining the fallback to nationalRegistry.


76-76: LGTM with a minor suggestion for consistency

The update from identityRegistry to identity is correct and maintains the existing functionality. The use of optional chaining and fallback to an empty string ensures type safety and a valid default value.

For consistency with the other fields and to prioritize the new identity data source, consider reordering the checks:

defaultValue: (application: ApplicantInformationInterface) => {
  return (
    application.externalData?.identity?.data?.address?.postalCode ??
    application.externalData?.nationalRegistry?.data?.address?.postalCode ??
    ''
  )
},

This change would prioritize the new identity data source while still maintaining the fallback to nationalRegistry.


89-89: LGTM with a minor suggestion for consistency

The update from identityRegistry to identity is correct and maintains the existing functionality. The use of optional chaining and fallback to an empty string ensures type safety and a valid default value.

For consistency with the other fields and to prioritize the new identity data source, consider reordering the checks:

defaultValue: (application: ApplicantInformationInterface) =>
  application.externalData?.identity?.data?.address?.city ??
  application.externalData?.nationalRegistry?.data?.address?.city ??
  '',

This change would prioritize the new identity data source while still maintaining the fallback to nationalRegistry.


Line range hint 1-114: Overall assessment: Changes look good with minor suggestions for improvement

The updates in this file consistently change the data source from identityRegistry to identity across multiple fields. These changes maintain the existing functionality, type safety, and fallback mechanisms.

Key points:

  1. The component remains reusable across different NextJS apps.
  2. TypeScript usage for defining props and exporting types is maintained.
  3. The changes don't negatively impact tree-shaking or bundling practices.

To further improve the code:

  1. Consider creating a helper function to handle the default value logic, reducing repetition across fields.
  2. Evaluate if the nationalRegistry fallback is still necessary. If not, consider removing it to simplify the code.
  3. If the identity structure is used across multiple components, consider creating a type definition for it to ensure consistency and improve maintainability.

These suggestions aim to enhance code reusability and maintainability while adhering to the coding guidelines for the libs directory.

libs/application/templates/accident-notification/src/lib/messages/externalData.ts (1)

75-84: LGTM! Consider adding a comment for clarity.

The new messages procureDescription and subTitle are well-structured and consistent with the existing code. They align with the PR objectives by providing information for procure holders.

Consider adding a brief comment above these new messages to explain their purpose and when they are used, for improved maintainability. For example:

// Messages specific to procure holders
procureDescription: {
  // ... (existing code)
},
subTitle: {
  // ... (existing code)
},
libs/application/templates/public-debt-payment-plan/src/fields/Overview/Overview.tsx (1)

141-148: LGTM with a minor suggestion

The changes correctly reflect the variable rename from identityRegistry to identity. The use of optional chaining in the condition is a good practice for safely accessing nested properties.

However, for consistency, consider using optional chaining in the text rendering as well:

- <Text>{identity.data.name}</Text>
+ <Text>{identity?.data?.name}</Text>

This ensures consistent null-safe property access throughout the component.

libs/application/templates/accident-notification/src/forms/PrerequsitesForm/externalDataSection.ts (4)

24-24: Remove unnecessary empty title property

The title property in the submitField is set to an empty string. If a title is not required, consider omitting this property to keep the code clean.


25-25: Review the necessity of refetchApplicationAfterSubmit

Confirm if refetchApplicationAfterSubmit: true is necessary. If the default behavior suffices or this property doesn't impact the desired functionality, consider removing it to simplify the code.


28-28: Use predefined event constants for consistency

Ensure that the event property uses a predefined constant or enum, such as DefaultEvents.SUBMIT, to maintain consistency and reduce the risk of typos.


Line range hint 11-33: Explicitly type the exported externalDataSection for clarity

To adhere to TypeScript best practices, consider explicitly typing the externalDataSection constant. Explicit types enhance code readability and maintainability.

libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (1)

67-67: Unused Parameter '_application'

The parameter _application in the reviewStatePendingAction function is prefixed with an underscore, indicating it's unused within the function body. If the parameter is not needed, consider removing it to clean up the code.

Alternatively, if maintaining the parameter for interface compliance or future use, keeping the underscore notation is acceptable.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between acd65c3 and 1c34a83.

📒 Files selected for processing (27)
  • libs/application/templates/accident-notification/src/constants/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/dataProviders/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/applicantInformationSection.ts (0 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/betaTestSection.ts (0 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/agreementDescriptionMultiField.ts (0 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/index.ts (0 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/index.ts (0 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/whoIsTheNotificationForMultiField.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/PrerequsitesForm/dataHandlingSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/PrerequsitesForm/externalDataSection.ts (2 hunks)
  • libs/application/templates/accident-notification/src/forms/PrerequsitesForm/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/externalDataSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (6 hunks)
  • libs/application/templates/accident-notification/src/lib/messages/applicantInformation.ts (1 hunks)
  • libs/application/templates/accident-notification/src/lib/messages/externalData.ts (2 hunks)
  • libs/application/templates/accident-notification/src/lib/messages/whoIsTheNotificationFor.ts (2 hunks)
  • libs/application/templates/accident-notification/src/types/index.ts (0 hunks)
  • libs/application/templates/accident-notification/src/utils/getWhoIstheNotificationForOptions.ts (1 hunks)
  • libs/application/templates/announcement-of-death/src/types/index.ts (0 hunks)
  • libs/application/templates/family-matters/children-residence-change-v2/src/types/index.ts (0 hunks)
  • libs/application/templates/general-fishing-license/src/dataProviders/index.ts (1 hunks)
  • libs/application/templates/general-fishing-license/src/lib/dataSchema.ts (1 hunks)
  • libs/application/templates/general-fishing-license/src/types/index.ts (0 hunks)
  • libs/application/templates/public-debt-payment-plan/src/fields/Overview/Overview.tsx (3 hunks)
  • libs/application/ui-forms/src/lib/applicantInformationMultiField/applicantInformationMultiField.ts (5 hunks)
  • libs/application/ui-forms/src/lib/applicantInformationMultiField/types.ts (1 hunks)
💤 Files with no reviewable changes (9)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/applicantInformationSection.ts
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/betaTestSection.ts
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/agreementDescriptionMultiField.ts
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/externalDataSection/index.ts
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/index.ts
  • libs/application/templates/accident-notification/src/types/index.ts
  • libs/application/templates/announcement-of-death/src/types/index.ts
  • libs/application/templates/family-matters/children-residence-change-v2/src/types/index.ts
  • libs/application/templates/general-fishing-license/src/types/index.ts
✅ Files skipped from review due to trivial changes (1)
  • libs/application/templates/accident-notification/src/dataProviders/index.ts
🧰 Additional context used
📓 Path-based instructions (17)
libs/application/templates/accident-notification/src/constants/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/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/whoIsTheNotificationForMultiField.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/accident-notification/src/forms/PrerequsitesForm/dataHandlingSection.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/accident-notification/src/forms/PrerequsitesForm/externalDataSection.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/accident-notification/src/forms/PrerequsitesForm/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/accident-notification/src/forms/PrerequsitesProcureForm/externalDataSection.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/accident-notification/src/forms/PrerequsitesProcureForm/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/accident-notification/src/lib/AccidentNotificationTemplate.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/accident-notification/src/lib/messages/applicantInformation.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/accident-notification/src/lib/messages/externalData.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/accident-notification/src/lib/messages/whoIsTheNotificationFor.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/accident-notification/src/utils/getWhoIstheNotificationForOptions.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/general-fishing-license/src/dataProviders/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/general-fishing-license/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/public-debt-payment-plan/src/fields/Overview/Overview.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/ui-forms/src/lib/applicantInformationMultiField/applicantInformationMultiField.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/ui-forms/src/lib/applicantInformationMultiField/types.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 (26)
libs/application/templates/accident-notification/src/constants/index.ts (1)

Line range hint 1-16: Code adheres to coding guidelines

The code in this file follows the coding guidelines for libs/**/* files:

  • It uses TypeScript for defining the enum and constants.
  • The constants and enum are likely reusable across different NextJS apps.
  • The simple structure of constants and enum supports effective tree-shaking and bundling practices.
libs/application/templates/accident-notification/src/forms/PrerequsitesForm/index.ts (1)

1-6: LGTM: Imports are well-structured and support reusability.

The imports are appropriately organized, separating external dependencies from local modules. The use of TypeScript types (Form) aligns with the coding guidelines for TypeScript usage.

libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/index.ts (1)

1-15: Overall, excellent implementation of the PrerequisitesProcureForm.

This file adheres to the coding guidelines for the libs/**/* pattern:

  • It promotes reusability by utilizing shared components and sections.
  • TypeScript is used effectively for type definitions.
  • The structure allows for easy integration into different NextJS apps.

The form is well-organized, properly typed, and follows project conventions. Great job!

libs/application/templates/accident-notification/src/utils/getWhoIstheNotificationForOptions.ts (2)

1-2: LGTM: Imports are appropriate and follow best practices.

The imports are well-structured and follow TypeScript naming conventions. They are sourced from relative paths within the same template, which is good for modularity and reusability across different NextJS apps.


1-32: Overall: Well-structured and adheres to coding guidelines.

This file is well-organized and follows TypeScript best practices. It effectively promotes reusability across different NextJS apps by exporting constants that provide a clear mapping between enum values and labels. The use of TypeScript for defining types (with room for improvement in explicit type annotations) enhances code quality and maintainability.

The file adheres to the coding guidelines for libs/**/* files:

  1. It promotes reusability of components across different NextJS apps.
  2. It uses TypeScript, though explicit type annotations could be added for better type safety.
  3. The exported constants are simple and should work well with tree-shaking and bundling practices.

The file maintains a single responsibility, which is excellent for maintainability and modularity. Overall, this is a solid addition to the codebase.

libs/application/templates/accident-notification/src/forms/PrerequsitesForm/dataHandlingSection.ts (4)

1-7: LGTM: Imports are well-structured and follow best practices.

The imports are concise and specific, which is beneficial for tree-shaking. The use of named imports from '@island.is/application/core' aligns with TypeScript best practices and promotes reusability across different NextJS apps.


8-11: LGTM: Section configuration is well-structured.

The section is properly configured using the buildSection function from the core library. The use of externalData for the title suggests good internationalization practices, promoting reusability across different contexts.


17-22: LGTM: Agreement description field is well-defined.

The custom field for the agreement description is properly configured using the buildCustomField function. The use of a custom 'AgreementDescription' component promotes modularity and reusability across different NextJS apps. Setting doesNotRequireAnswer to true is appropriate for a description field.


1-40: Great job! The file is well-structured and adheres to best practices.

This new file successfully creates a data handling section for the accident notification form, adhering to the coding guidelines for the libs directory. Key strengths include:

  1. Effective use of the application core library, promoting reusability across different NextJS apps.
  2. Clear and consistent structure, enhancing maintainability.
  3. Good use of TypeScript, with potential for further type safety improvements.
  4. Support for internationalization through the use of externalData.

The code is of high quality and ready for integration into the project.

libs/application/templates/accident-notification/src/forms/PrerequsitesProcureForm/externalDataSection.ts (4)

1-10: LGTM: Imports and module structure are well-organized.

The imports from '@island.is/application/core' promote reusability across NextJS apps, aligning with our coding guidelines. The local imports for messages and IdentityApi demonstrate good modularity.


11-14: LGTM: Well-structured externalDataSection constant.

The exported constant uses the buildSection utility, promoting reusability and adhering to our coding guidelines. The structure is clear and well-defined, facilitating easy integration with other parts of the application.


1-48: Overall, well-structured and adherent to coding guidelines.

This new file introduces an external data section for a form, following good practices for reusability and modularity. It aligns well with the coding guidelines for the libs directory, particularly in terms of component reusability and TypeScript usage. While there are minor suggestions for improvement in TypeScript type annotations, the overall implementation is solid and contributes positively to the accident notification feature.


15-46: LGTM: Well-configured buildExternalDataProvider with a minor query.

The configuration is comprehensive and well-structured, using localized messages for text content. The submit field configuration, including the refetch after submit, is a good practice for maintaining data consistency.

Could you clarify the status of the commented-out NationalRegistryUserApi provider? Is this planned for future implementation or a removed feature? If it's no longer needed, consider removing the commented code to keep the file clean.

To verify the usage of NationalRegistryUserApi, you can run the following script:

✅ Verification successful

LGTM: buildExternalDataProvider configuration is well-designed.

NationalRegistryUserApi is utilized extensively across the codebase, ensuring its critical role in the application. The commented-out provider in externalDataSection.ts appears to be unnecessary and can be removed to maintain code cleanliness.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of NationalRegistryUserApi in the codebase

# Test: Search for NationalRegistryUserApi usage
rg --type typescript "NationalRegistryUserApi"

Length of output: 81


Script:

#!/bin/bash
# Description: Check for other occurrences of NationalRegistryUserApi in TypeScript files

# Test: Search for NationalRegistryUserApi usage in .ts and .tsx files
rg "NationalRegistryUserApi" --glob "*.ts" --glob "*.tsx"

Length of output: 29810

libs/application/templates/accident-notification/src/lib/messages/applicantInformation.ts (1)

Line range hint 1-69: Adherence to coding guidelines for libs/**/*

The changes in this file adhere to the coding guidelines for the libs/**/* pattern:

  1. Reusability: The applicantInformation object is exported and can be reused across different NextJS apps.
  2. TypeScript usage: The code uses TypeScript for defining the structure of the exported object.
  3. Tree-shaking and bundling: The use of named exports (export const applicantInformation) allows for effective tree-shaking in the bundling process.
libs/application/templates/accident-notification/src/lib/messages/whoIsTheNotificationFor.ts (1)

Line range hint 1-53: Overall, the changes adhere to coding guidelines and maintain good practices.

The additions to this file:

  1. Maintain reusability across different NextJS apps.
  2. Use TypeScript effectively for defining message structures.
  3. Follow existing patterns, supporting effective tree-shaking and bundling.
  4. Adhere to the established internationalization approach using react-intl.

These changes align well with the coding guidelines for files in the libs directory.

libs/application/templates/general-fishing-license/src/lib/dataSchema.ts (1)

Line range hint 19-29: LGTM! Verify consistency across the codebase.

The renaming of 'identityRegistry' to 'identity' looks good and maintains the existing structure. This change adheres to the coding guidelines for TypeScript usage and schema definition.

To ensure consistency, please verify that this renaming has been applied uniformly across the codebase. Run the following script to check for any remaining instances of 'identityRegistry':

If the script returns any results, consider updating those instances to maintain consistency.

✅ Verification successful

Consistency Verified

The renaming of 'identityRegistry' to 'identity' in the data schema is correctly applied and does not affect the localization message keys in externalData.ts. No further changes are necessary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for remaining instances of 'identityRegistry'

# Search for 'identityRegistry' in TypeScript and JavaScript files
rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js 'identityRegistry'

Length of output: 637


Script:

#!/bin/bash
# Description: Display lines surrounding each occurrence of 'identityRegistry' in externalData.ts

# Display 2 lines before and after each match for context
rg -C 2 'identityRegistry' libs/application/templates/public-debt-payment-plan/src/lib/messages/externalData.ts

Length of output: 599

libs/application/templates/accident-notification/src/lib/messages/externalData.ts (1)

Line range hint 1-84: Adherence to coding guidelines confirmed.

The code in this file follows the specified guidelines for libs/**/* files:

  1. It supports reusability across different NextJS apps by exporting the externalData object.
  2. TypeScript is used effectively for defining the message objects.
  3. The structure of the file, with separate message definitions, supports effective tree-shaking and bundling.
libs/application/templates/public-debt-payment-plan/src/fields/Overview/Overview.tsx (4)

56-58: LGTM: Improved variable naming

The change from identityRegistry to identity improves code readability while maintaining the same functionality and type safety. This rename is consistent throughout the file and aligns with best practices for clear and concise variable naming.


161-163: LGTM: Improved null-safety

The conditions for rendering the address have been updated correctly to use identity?.data?.address. The consistent use of optional chaining for all nested properties enhances the null-safety of the code, reducing the risk of runtime errors due to undefined properties.


166-166: LGTM: Consistent null-safe property access

The address rendering has been updated to use identity?.data?.address consistently. This change maintains the improved null-safety introduced in the condition check and ensures that the component handles potential undefined values gracefully.


Line range hint 1-300: Overall assessment: Approved with minor suggestions

The changes in this file consistently rename identityRegistry to identity and improve null-safety through the use of optional chaining. These modifications enhance code readability and reduce the risk of runtime errors.

The component adheres to the coding guidelines for reusability and TypeScript usage. It effectively uses TypeScript for prop definitions and type exports, promoting type safety throughout the application.

A minor suggestion was made to use optional chaining consistently in the name rendering, but this doesn't impact the overall functionality or safety of the code.

The changes are approved, and the code is ready for the next stage of the development process.

libs/application/templates/accident-notification/src/forms/PrerequsitesForm/externalDataSection.ts (4)

4-6: Imports added are appropriate and necessary

The added imports buildSection, buildSubmitField, and coreMessages are required for the new functionality and are correctly imported from '@island.is/application/core'.


8-9: Updated import paths correctly reference new locations

The import paths for externalData and NationalRegistryUserApi have been updated to reflect their correct locations, ensuring proper module resolution.


21-33: Confirm submitField integration within buildExternalDataProvider

The addition of the submitField property within buildExternalDataProvider is a significant change. Verify that buildExternalDataProvider supports the submitField property and that this integration aligns with the application's form architecture.

Consider checking the documentation or existing patterns to ensure this addition follows best practices. If submitField is not a supported property, you may need to adjust the implementation.


11-12: Verify all references to renamed externalDataSection

The exported constant has been renamed from accidentNotificationSubSection to externalDataSection, and the id updated to 'ExternalDataRegularSection'. Please ensure all references to the old names are updated throughout the codebase to prevent any broken imports or references.

You can run the following script to check for any outdated references:

✅ Verification successful

All references to accidentNotificationSubSection have been successfully updated. No broken imports or references detected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find references to 'accidentNotificationSubSection' and 'AccidentNotificationForm' in the codebase.

rg 'accidentNotificationSubSection|AccidentNotificationForm'

Length of output: 986

libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (1)

99-103: Compliance with Coding Guidelines for TypeScript Types

The addition of allowedDelegations with specific AuthDelegationType enhances delegation capabilities. Ensure that all types are properly defined and exported to maintain TypeScript standards.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (1)
libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (1)

97-104: LGTM: Configuration updates for delegations and initial state

The addition of allowedDelegations and the change of the initial state to PREQUISITES align with the PR objectives. These updates enable the new procure functionality and introduce a new starting point for the application flow.

There's a minor typo in the state name. Consider changing PREQUISITES to PREREQUISITES for better clarity and correctness.

Also applies to: 106-106

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1c34a83 and c9fe8ed.

📒 Files selected for processing (1)
  • libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.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 (4)
libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (4)

18-18: LGTM: New imports and PROCURER role added

The additions of FormModes, AuthDelegationType, and the new PROCURER role in the Roles enum align well with the PR objectives. These changes lay the groundwork for implementing the new functionality allowing users with procure to submit accident notifications.

Also applies to: 28-29, 35-35


185-197: LGTM: PROCURER role added to DRAFT state

The addition of the PROCURER role to the DRAFT state, with the same form loader as the APPLICANT, aligns with the PR objectives. This change allows procurers to access and fill out the same form as applicants, enhancing the flexibility of the application process.


Line range hint 1-468: Compliance with coding guidelines confirmed

The file adheres to the coding guidelines for the libs/**/* pattern:

  • TypeScript is used effectively for defining props and exporting types.
  • Components and hooks appear to be designed for reusability across different NextJS apps.
  • No obvious issues with tree-shaking or bundling practices are observed.

Line range hint 1-468: Overall assessment: Well-implemented procure functionality

The changes in this file successfully implement the new procure functionality as outlined in the PR objectives. The addition of the PROCURER role, new states, and updated configurations enhance the application's ability to handle accident notifications submitted by users with procure.

Key points:

  1. New PREQUISITES state added with role-specific configurations.
  2. PROCURER role integrated into existing states.
  3. mapUserToRole function updated to handle the new role.

While some minor issues were identified (typos, role assignment logic), the overall implementation is well-structured and aligns with the project's goals. After addressing the suggested improvements, this implementation will provide a solid foundation for the new accident notification submission process.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (10)
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/powerOfAttorneyUploadSubSection.ts (1)

Line range hint 11-45: Consider enhancing type safety for the file upload configuration.

While the implementation is correct, we could improve type safety for the file upload configuration.

Consider creating a type-safe constant for the upload configuration:

interface FileUploadConfig {
  maxSize: number;
  acceptedTypes: string[];
}

export const POWER_OF_ATTORNEY_UPLOAD_CONFIG: FileUploadConfig = {
  maxSize: FILE_SIZE_LIMIT,
  acceptedTypes: UPLOAD_ACCEPT,
} as const;

This would provide better type checking and make the configuration more maintainable.

libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/juridicialPersonCompanySubSection.ts (1)

Line range hint 11-57: Consider enhancing type safety and documentation.

The form implementation is well-structured using reusable builders and proper validation. However, consider these improvements:

  1. Add TypeScript interfaces for the form values
  2. Document the expected format for companyNationalId
  3. Add JSDoc comments explaining the condition function's logic

Example type definitions:

interface JuridicalPersonFormValues {
  companyName: string;
  companyNationalId: string;
  companyConfirmation: string[];
}

/**
 * Builds the juridical person company subsection of the form.
 * This section is only shown when the user is reporting on behalf of an employee.
 * @param formValue The complete form values
 * @returns boolean indicating if the section should be shown
 */
libs/application/templates/accident-notification/src/utils/getWorkplaceData.spec.ts (1)

Line range hint 1-89: Consider adding error case tests for improved coverage.

While the current test suite covers the happy paths well, consider adding tests for:

  • Invalid accident types
  • Malformed input objects
  • Edge cases where required nested properties are missing

This would help ensure the utility function handles error cases gracefully.

Example test cases to add:

it('should handle invalid accident type gracefully', () => {
  const invalidAccident: FormValue = {
    accidentType: { radioButton: 'INVALID_TYPE' as AccidentTypeEnum },
  }
  expect(getWorkplaceData(invalidAccident)?.type).toBeUndefined()
})

it('should handle missing required nested properties', () => {
  const incompleteAccident: FormValue = {
    accidentType: {} // missing radioButton
  }
  expect(getWorkplaceData(incompleteAccident)?.type).toBeUndefined()
})
libs/application/templates/accident-notification/src/fields/DateOfAccident/index.tsx (2)

Line range hint 52-59: Consider enhancing error handling for health insurance status checks.

The current implementation only logs errors to the console. Users won't be notified if the health insurance status check fails, which could lead to confusion.

Consider implementing user feedback:

 .catch((err) => {
   console.log(
     'An error occured fetching health insurance status: ',
     err,
   )
+  // Show error message to user
+  setValue('accidentDetails.isHealthInsured', '')
+  // Optionally, use a toast or error message component
 })

Line range hint 89-108: Simplify form control implementation.

The current implementation uses both register and Controller which is redundant. Consider simplifying to use just Controller.

Suggested implementation:

-      <Controller
-        name="reason"
-        defaultValue={application.answers.residenceChangeReason}
-        render={({ field: { onChange } }) => {
-          return (
-            <Input
-              {...register('accidentDetails.isHealthInsured', {
-                onChange: (e) => {
-                  onChange(e.target.value)
-                  setValue('accidentDetails.isHealthInsured', e.target.value)
-                },
-              })}
-              defaultValue={'yes'}
-              value={isHealthInsured}
-            />
-          )
-        }}
-      />
+      <Controller
+        name="accidentDetails.isHealthInsured"
+        defaultValue={isHealthInsured || 'yes'}
+        render={({ field }) => (
+          <Input {...field} type="hidden" />
+        )}
+      />
libs/application/templates/accident-notification/src/utils/hasMissingDocuments.spec.ts (1)

Line range hint 1-120: Well-structured test suite with comprehensive coverage.

The test suite follows best practices:

  • Clear test organization with descriptive test cases
  • Good use of TypeScript for type safety
  • Proper test data factories
  • Comprehensive coverage of different scenarios

Consider extracting the test data factories (getMissingDocuments, getNoMissingDocuments) into a shared test utilities file if they're used across multiple test files, promoting reusability across the test suite.

libs/application/templates/accident-notification/src/types/index.ts (2)

Line range hint 42-46: Consider documenting the FileType interface.

The FileType interface contains an optional url property without clear documentation about when it might be undefined.

Add JSDoc comments to clarify the usage:

+/**
+ * Represents a file attachment in the accident notification system
+ * @property url - Optional URL to the file, undefined when file is not yet uploaded
+ * @property name - Display name of the file
+ * @property key - Unique identifier for the file
+ */
 export type FileType = {
   url?: string | undefined
   name: string
   key: string
 }

Line range hint 89-91: Standardize enum value string literals.

Some enum values contain their parent enum name as a prefix in the string literal, while others don't. This inconsistency could make maintenance more difficult.

Consider standardizing the enum values:

 export enum GeneralWorkplaceAccidentLocationEnum {
   ATTHEWORKPLACE = 'atTheWorkplace',
-  TOORFROMTHEWORKPLACE = 'GeneralWorkplaceAccidentLocation.toOrFromTheWorkplace',
-  OTHER = 'GeneralWorkplaceAccidentLocation.other',
+  TOORFROMTHEWORKPLACE = 'toOrFromTheWorkplace',
+  OTHER = 'other',
 }

Also applies to: 96-98, 103-105

libs/application/templates/accident-notification/src/lib/dataSchema.ts (1)

Line range hint 1-300: Consider extracting common validation patterns for reuse.

To improve reusability across different NextJS apps, consider extracting frequently used validation patterns into shared utilities:

  1. Common string validations (non-empty, email)
  2. Kennitala validations for both person and company
  3. Time format validations

Example implementation:

// libs/shared/validation/src/common.ts
export const commonValidators = {
  nonEmptyString: (errorMsg: string) =>
    z.string().refine((x) => x.trim().length > 0, {
      params: errorMsg,
    }),
  
  kennitalaPerson: (errorMsg: string) =>
    z.string().refine((x) => (x ? kennitala.isPerson(x) : false), {
      params: errorMsg,
    }),

  kennitalaCompany: (errorMsg: string) =>
    z.string().refine((x) => (x ? kennitala.isCompany(x) : false), {
      params: errorMsg,
    }),
    
  time24h: (errorMsg: string) =>
    z.string().refine((x) => (x ? isValid24HFormatTime(x) : false), {
      params: errorMsg,
    })
}
libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/attachmentsSubSection.ts (1)

Line range hint 1-215: Excellent adherence to library code guidelines.

The code demonstrates:

  • ✓ Reusable form components and hooks
  • ✓ Strong TypeScript typing with proper exports
  • ✓ Effective tree-shaking through named exports

Consider extracting the file size validation logic into a shared hook for reuse across different file upload components in the application.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between c9fe8ed and 42fa271.

📒 Files selected for processing (23)
  • libs/application/templates/accident-notification/src/fields/DateOfAccident/index.tsx (1 hunks)
  • libs/application/templates/accident-notification/src/fields/FormOverview/index.tsx (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/attachmentsSubSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/locationSubSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/workMachineSubSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/juridicialPersonCompanySubSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/powerOfAttorneyUploadSubSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/forms/InReviewForm/addAttachmentsSection.ts (1 hunks)
  • libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (5 hunks)
  • libs/application/templates/accident-notification/src/lib/dataSchema.ts (1 hunks)
  • libs/application/templates/accident-notification/src/types/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/constants/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/getWorkplaceData.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/hasMissingDocuments.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/hasMissingDocuments.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/index.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/index.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isFatalAccident.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isFatalAccident.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isMachineRelatedAccident.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isMachineRelatedAccident.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isRepresentativeOfCompanyOrInstitute.spec.ts (1 hunks)
  • libs/application/templates/accident-notification/src/utils/isRepresentativeOfCompanyOrInstitute.ts (1 hunks)
✅ Files skipped from review due to trivial changes (10)
  • libs/application/templates/accident-notification/src/fields/FormOverview/index.tsx
  • libs/application/templates/accident-notification/src/forms/InReviewForm/addAttachmentsSection.ts
  • libs/application/templates/accident-notification/src/utils/hasMissingDocuments.ts
  • libs/application/templates/accident-notification/src/utils/index.spec.ts
  • libs/application/templates/accident-notification/src/utils/index.ts
  • libs/application/templates/accident-notification/src/utils/isFatalAccident.spec.ts
  • libs/application/templates/accident-notification/src/utils/isFatalAccident.ts
  • libs/application/templates/accident-notification/src/utils/isMachineRelatedAccident.spec.ts
  • libs/application/templates/accident-notification/src/utils/isRepresentativeOfCompanyOrInstitute.spec.ts
  • libs/application/templates/accident-notification/src/utils/isRepresentativeOfCompanyOrInstitute.ts
🧰 Additional context used
📓 Path-based instructions (13)
libs/application/templates/accident-notification/src/fields/DateOfAccident/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/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/attachmentsSubSection.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/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/locationSubSection.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/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/workMachineSubSection.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/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/juridicialPersonCompanySubSection.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/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/powerOfAttorneyUploadSubSection.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/accident-notification/src/lib/AccidentNotificationTemplate.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/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/accident-notification/src/types/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/accident-notification/src/utils/constants/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/accident-notification/src/utils/getWorkplaceData.spec.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/accident-notification/src/utils/hasMissingDocuments.spec.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/accident-notification/src/utils/isMachineRelatedAccident.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 (25)
libs/application/templates/accident-notification/src/utils/constants/index.ts (2)

Line range hint 1-15: LGTM on exports and TypeScript usage!

The constants and enums are properly exported for reuse across the application, following the coding guidelines for libs/. The TypeScript enum implementation allows for proper tree-shaking.


10-10: Verify state machine transitions for the new prerequisites state

The addition of this new state before DRAFT suggests it's an initial state in the flow. Let's verify all state transitions are properly handled.

✅ Verification successful

Let me search for the state machine configuration and transitions for this specific template.


Let me search for the complete state machine configuration to see all transitions.


Let me try a different approach to search for the state machine configuration.


Let me try one final search to understand the state transitions.


Based on the gathered information, I can now provide a final response.

State transitions for prerequisites state appear properly configured

The state machine configuration shows that:

  • PREREQUISITES is correctly set as the initial state
  • It has proper lifecycle configuration with FormModes.DRAFT status
  • It includes proper role configuration (APPLICANT)
  • It has the expected SUBMIT event handler for transitioning to the next state
  • The state flow follows a logical progression: PREREQUISITES -> DRAFT -> REVIEW -> REVIEW_ADD_ATTACHMENT -> IN_FINAL_REVIEW

The implementation follows the standard pattern used across other templates in the codebase that also have prerequisites states, and includes proper event handling for state transitions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for state machine configuration and transitions
rg -A 5 "States\.PREQUISITES|'prerequisites'" --type ts

Length of output: 35715


Script:

#!/bin/bash
# Search for state machine configuration in the accident notification template
rg -A 15 "initial: States\.PREQUISITES" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

# Search for state transitions
ast-grep --pattern 'on: { type: $_ }'

Length of output: 649


Script:

#!/bin/bash
# Get the full state machine configuration
rg -A 50 "machine: {" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

# Also search for any event type definitions
rg "type Events" -A 10 libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

Length of output: 310


Script:

#!/bin/bash
# Search for the state machine configuration with proper escaping
rg -A 50 "machine = {|machine: {" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

# Search for transitions from PREREQUISITES state
rg -B 2 -A 5 "\[States\.PREQUISITES\].*on: {" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

# Search for all state definitions to understand the flow
rg "\[States\.[A-Z_]+\]" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

Length of output: 770


Script:

#!/bin/bash
# Search for the content around PREREQUISITES state with more context
rg -B 5 -A 20 "\[States\.PREQUISITES\]" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

# Search for any transition events defined in the file
rg "\.SUBMIT|\.APPROVE|\.REJECT|\.NEXT|\.PREVIOUS" libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts

Length of output: 1929

libs/application/templates/accident-notification/src/utils/isMachineRelatedAccident.ts (1)

Line range hint 1-12: LGTM! Well-structured TypeScript implementation.

The implementation follows best practices with proper typing, named imports for better tree-shaking, and a clear, reusable utility function structure.

libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/whoIsTheNotificationForSection/powerOfAttorneyUploadSubSection.ts (1)

8-8: Verify consistency of constants relocation.

The import path change aligns with the centralization of constants. Let's verify this change is consistent across the codebase.

✅ Verification successful

Let me gather more information about the constants and their location to better understand the situation.


Import path is correct and consistent with codebase structure

The verification shows that:

  • The constants are correctly defined in src/utils/constants/index.ts
  • The import path ../../../utils/constants in the file under review is correct and matches the actual location
  • All other files in the codebase are consistently using the same import pattern
  • The constants are properly exported and used across multiple form sections
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all constant imports are consistently using the new path
# and no references to the old path remain

# Test 1: Check if any files still import from the old constants path
echo "Checking for remaining imports from old path..."
rg "from '\.\./\.\./\.\./constants'" libs/application/templates/accident-notification/src/

# Test 2: Verify the new constants file exists and contains the expected constants
echo "Verifying new constants location..."
rg "export const (FILE_SIZE_LIMIT|UPLOAD_ACCEPT)" libs/application/templates/accident-notification/src/utils/constants.ts

Length of output: 520


Script:

#!/bin/bash
# Let's first find where these constants actually exist
echo "Searching for FILE_SIZE_LIMIT constant..."
rg "FILE_SIZE_LIMIT" libs/application/templates/accident-notification/

echo -e "\nSearching for UPLOAD_ACCEPT constant..."
rg "UPLOAD_ACCEPT" libs/application/templates/accident-notification/

echo -e "\nListing all constant.ts files in the accident-notification template..."
fd "constants.ts" libs/application/templates/accident-notification/

Length of output: 4759

libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/workMachineSubSection.ts (1)

13-13: LGTM: Import path update is consistent with codebase changes.

The change to import constants from utils/constants aligns with the module organization patterns seen in other files.

Let's verify the consistency of this change across the codebase:

✅ Verification successful

Import path change is correctly implemented and consistent

The verification shows that within the accident-notification template, all imports of YES/NO constants are consistently using the new path utils/constants. This is evidenced by multiple files in the template using the same import pattern:

  • src/types/index.ts
  • src/lib/dataSchema.ts
  • src/fields/FormOverview/index.tsx
  • src/fields/DateOfAccident/index.tsx
  • src/forms/AccidentNotificationForm/*/*.ts

The change aligns with the module's internal organization and doesn't affect other templates or applications that maintain their own constants files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all imports of YES/NO constants use the new path

# Test 1: Check for any remaining imports from the old path
echo "Checking for old import paths..."
rg "import.*{.*(?:YES|NO).*}.*from.*['\"].*constants['\"]" --type ts

# Test 2: Verify consistent usage of new import path
echo "Checking new import paths..."
rg "import.*{.*(?:YES|NO).*}.*from.*['\"].*utils/constants['\"]" --type ts

Length of output: 589


Script:

#!/bin/bash
# Let's try a simpler regex pattern to find YES/NO imports

echo "Checking imports from 'constants'..."
rg "from ['\"].*constants['\"]" --type ts -B1 | grep -E "YES|NO"

echo -e "\nChecking imports from 'utils/constants'..."
rg "from ['\"].*utils/constants['\"]" --type ts -B1 | grep -E "YES|NO"

Length of output: 17232

libs/application/templates/accident-notification/src/utils/getWorkplaceData.spec.ts (1)

1-3: LGTM! Import path update looks correct.

The change to import YES from './constants' aligns with the file reorganization mentioned in the summary.

libs/application/templates/accident-notification/src/fields/DateOfAccident/index.tsx (2)

8-8: LGTM! Import path change and component structure comply with library guidelines.

The component follows best practices for reusability and proper TypeScript usage.


Line range hint 77-78: Verify date range requirements for accident notifications.

The current implementation allows dates from 1970 to present. Please verify if this aligns with business requirements for accident reporting timeframes.

✅ Verification successful

Let me try a different search strategy to find date validation requirements.

Let's check if we can find any validation rules or test cases that might provide insights into the date requirements for accident notifications.


Let me try one more search to find any validation schema or business rules specifically for the accident notification form.

This should help us find any validation rules or test cases that might provide insights into the date requirements.


Based on the code analysis from previous searches, I can now provide a final response about the date validation requirements:

Date range implementation appears reasonable for accident reporting

The current implementation with date range from 1970 to present is appropriate for accident notifications because:

  1. The minimum date of 1970 provides sufficient historical coverage for reporting past accidents while preventing invalid dates before the Unix epoch
  2. The maximum date being set to the current date (maxDate={new Date()}) prevents future-dated accident reports
  3. No evidence was found of specific business rules requiring a shorter timeframe

This aligns with common accident reporting practices where:

  • Historical accidents should be reportable for legal/insurance purposes
  • Future dates should be prevented as accidents cannot be reported before they occur
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any date validation requirements in documentation or tests
rg -g '!node_modules' -g '*.{md,txt,test.*,spec.*}' -i 'accident.*date.*validation|accident.*reporting.*period'

Length of output: 113


Script:

#!/bin/bash
# Search for test files related to accident notification and date validation
fd "accident.*\.(test|spec)\.(ts|tsx|js|jsx)$"

# Search for any date validation or constraints in the accident notification component
rg -g '!node_modules' "minDate|maxDate|DateOfAccident" -A 3 -B 3

# Look for any validation schema or rules
ast-grep --pattern 'const $_ = yup.object().shape({
  $$$
  dateOfAccident: $$$,
  $$$
})'

Length of output: 150539


Script:

#!/bin/bash
# Search for validation schema in accident notification related files
rg -g '!node_modules' -g '*.{ts,tsx}' "yup.*dateOfAccident|validation.*accident.*date" -A 3 -B 3

# Look for any test cases or business rules around accident date
rg -g '!node_modules' -g '*.{ts,tsx}' "describe.*accident.*date|test.*accident.*date" -A 5 -B 5

Length of output: 196

libs/application/templates/accident-notification/src/utils/hasMissingDocuments.spec.ts (1)

7-7: Verify the constants module location change.

The import path change from '../constants' to './constants' suggests a module reorganization. This change maintains better module cohesion by keeping related utilities together.

Let's verify the module organization:

libs/application/templates/accident-notification/src/types/index.ts (2)

1-1: LGTM! Import statement follows best practices.

The import statement is specific and uses a maintainable path format.


Line range hint 183-241: Enhance type safety for V2 types.

The new V2 types have several potential issues:

  1. All fields being optional could lead to runtime issues
  2. Lack of documentation about migration from V1 to V2
  3. Nullable fields might cause unnecessary null checks

Let's verify the usage of these types:

Consider these improvements:

  1. Add documentation about V2 types:
/**
 * V2 types for the new accident notification API
 * @remarks
 * These types represent the new API contract with optional fields
 * to support partial data submission.
 */
  1. Consider using discriminated unions for better type safety:
type ApplicantV2 = {
  type: 'complete'
  address: string
  city: string
  // ... other required fields
} | {
  type: 'partial'
  address?: string | null
  city?: string | null
  // ... other optional fields
}
libs/application/templates/accident-notification/src/lib/dataSchema.ts (2)

Line range hint 1-300: Strong TypeScript implementation with comprehensive validation.

The schema definitions demonstrate excellent TypeScript usage with:

  • Proper type exports using Zod's TypeOf
  • Comprehensive validation rules with custom error messages
  • Well-structured nested schemas for different aspects of accident notifications

4-4: Verify constant relocation impact.

The YES constant has been moved from ../constants to ../utils/constants. Let's ensure this change is consistent across the codebase.

libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/attachmentsSubSection.ts (1)

24-29: LGTM! Import restructuring aligns with best practices.

The reorganization of constants into the utils/constants module improves code organization and maintainability.

Let's verify that this import path change is consistent across the codebase:

✅ Verification successful

Let me try a different approach to verify the import changes more thoroughly.


Based on the results, I notice that the constants are actually located in a subdirectory at utils/constants/index.ts. Let me verify the content of this file.


Import changes are correctly implemented and verified

The verification results show that:

  1. The constants (FILE_SIZE_LIMIT, NO, UPLOAD_ACCEPT, YES) are properly defined in utils/constants/index.ts
  2. The import statement in attachmentsSubSection.ts correctly references these constants from ../../../utils/constants
  3. No files are using the old import path ../../../constants
  4. All other files in the codebase are consistently using the new utils/constants path
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if any files still import constants from the old path
# Expect: No results, indicating all imports have been updated

rg -l "from '\.\./\.\./\.\./constants'" libs/application/templates/accident-notification/

Length of output: 97


Script:

#!/bin/bash
# Search for any imports from 'constants' or 'utils/constants' in the accident-notification directory
echo "=== Checking imports patterns ==="
rg "from.*['\"].*constants['\"]" libs/application/templates/accident-notification/ -A 1

# List all files in the utils directory to confirm the constants file location
echo -e "\n=== Checking utils directory structure ==="
fd . libs/application/templates/accident-notification/src/utils/

# Check the content of the constants file to verify it contains the imported constants
echo -e "\n=== Checking constants file content ==="
rg "FILE_SIZE_LIMIT|NO|UPLOAD_ACCEPT|YES" libs/application/templates/accident-notification/src/utils/constants.ts

Length of output: 12797


Script:

#!/bin/bash
# Check the content of the constants file in the correct location
echo "=== Checking constants in the correct path ==="
cat libs/application/templates/accident-notification/src/utils/constants/index.ts

# Verify all imports are using the correct path
echo -e "\n=== Checking for any old-style imports ==="
rg "from '\.\./\.\./\.\./constants'" libs/application/templates/accident-notification/

# Double-check imports specifically in the attachmentsSubSection file
echo -e "\n=== Checking specific file imports ==="
cat libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/attachmentsSubSection.ts

Length of output: 9094

libs/application/templates/accident-notification/src/forms/AccidentNotificationForm/aboutTheAccidentSection/locationSubSection.ts (2)

Line range hint 1-400: LGTM! The code follows best practices and guidelines.

The implementation demonstrates:

  • Proper use of TypeScript with well-defined enums and types
  • Reusable form builder components
  • Clear conditional rendering logic
  • Effective tree-shaking through named imports/exports

25-25: Verify the constants file location.

The import path has been updated to source constants from the utils directory. Let's verify that this file exists and other files are consistently using the same path.

✅ Verification successful

Let me generate another verification script to check the actual location and content of the constants file since the first results don't show its existence.


Let me do one final check to locate the YES and NO constants since they weren't found in the shared constants file.


Based on the shell script results, I can now provide a final response regarding the constants import verification:

Import path change is correct and consistent

The verification shows that:

  1. The YES and NO constants are correctly defined in libs/application/templates/accident-notification/src/utils/constants/index.ts
  2. Multiple files are consistently using the new import path '../../../utils/constants'
  3. No files are using the old import path

The import path change aligns with the actual location of the constants and follows the established pattern across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of constants file and its usage across the codebase

# Test 1: Check if the constants file exists
echo "Checking for constants file..."
fd --type f "constants.ts" "libs/application/templates/accident-notification/src/utils"

# Test 2: Check for any remaining imports from the old path
echo "Checking for old import paths..."
rg "from '\.\./\.\./\.\./constants'" "libs/application/templates/accident-notification"

# Test 3: Check for consistency in new import paths
echo "Checking for new import paths..."
rg "from '\.\./\.\./\.\./utils/constants'" "libs/application/templates/accident-notification"

Length of output: 1455


Script:

#!/bin/bash
# Search for any constants.ts file in the accident-notification directory
echo "Searching for constants.ts in the entire accident-notification directory..."
fd --type f "constants.ts" "libs/application/templates/accident-notification"

# Check the content of any found constants file to verify it contains YES and NO
echo "Checking content of constants files..."
fd --type f "constants.ts" "libs/application/templates/accident-notification" --exec cat {}

Length of output: 2534


Script:

#!/bin/bash
# Search for YES and NO constants definitions in the entire accident-notification directory
echo "Searching for YES/NO constants definitions..."
rg "export const (?:YES|NO) =" "libs/application/templates/accident-notification"

# Also check if these constants might be in a utils directory
echo "Checking utils directory specifically..."
fd --type f . "libs/application/templates/accident-notification/src/utils" --exec cat {}

Length of output: 76287

libs/application/templates/accident-notification/src/lib/AccidentNotificationTemplate.ts (9)

18-18: Importing 'FormModes' for Form Status Management

The inclusion of FormModes is appropriate as it is used to define the form status in the application states.


23-23: Importing 'States' Enum Enhances Readability

Importing States from constants helps maintain clear and consistent state definitions across the application.


28-29: Appropriate Imports for Delegations and Data Access

The addition of AuthDelegationType, IdentityApi, and NationalRegistryUserApi imports is necessary for handling delegation types and accessing user data.


35-38: Addition of 'PROCURER' Role in Roles Enum

Including PROCURER in the Roles enum correctly extends role functionality to support procurer users.


43-44: Extending 'AccidentNotificationEvent' with 'SUBMIT' and 'ASSIGN'

Adding these events allows the state machine to handle submission and assignment actions appropriately.


67-69: Using Underscore Prefix for Unused Parameter

Renaming application to _application indicates the parameter is intentionally unused, adhering to best practices to prevent linting warnings.


97-104: Defining 'allowedDelegations' Enhances Security

Specifying allowedDelegations with ProcurationHolder and Custom types explicitly outlines permitted delegation types, improving security and clarity.


106-106: Setting Initial State to 'States.PREQUISITES'

Changing the initial state to States.PREQUISITES aligns with the new user flow, ensuring prerequisites are addressed first.


185-197: Including 'PROCURER' Role in 'States.DRAFT'

Adding the PROCURER role to the DRAFT state allows procurers to participate in drafting the accident notification, enhancing functionality.

@jonnigs jonnigs added the automerge Merge this PR as soon as all checks pass label Dec 11, 2024
Copy link
Member

@HjorturJ HjorturJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@kksteini kksteini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowner files lgtm

@kodiakhq kodiakhq bot merged commit 057fb9e into main Dec 12, 2024
540 checks passed
@kodiakhq kodiakhq bot deleted the feat/accident-notification-procure branch December 12, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR as soon as all checks pass deploy-feature Deploys features to dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants