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(rental-agreement-application): Other fees subsection #16650

Merged
merged 8 commits into from
Oct 31, 2024

Conversation

hebaulf
Copy link

@hebaulf hebaulf commented Oct 30, 2024

Other Fees Subsection

Asana Ticket

What

  • Add fields needed for other fees section
  • Validate needed fields
  • Update rentalAmount section, remove date field for indexation

Why

A contracting party should be able to define how the distribution of fees not included in the rental price should be divided between the landlord and the tenant.

Screenshots / Gifs

Screenshot 2024-10-30 at 13 52 22

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 a new rental agreement template, enhancing the application's capability to manage rental agreements.
    • Added support for external data integration within the rental agreement process.
  • Documentation

    • Added README and other documentation files to guide users on utilizing the rental agreement features.
  • Internationalization

    • Implemented internationalized messages for various components related to rental agreements, improving user accessibility.
  • Configuration

    • Updated TypeScript and Babel configurations to support the new rental agreement module.
  • Enhancements

    • Improved data validation and error handling for rental agreement forms.

@hebaulf hebaulf requested review from a team as code owners October 30, 2024 13:53
Copy link
Contributor

coderabbitai bot commented Oct 30, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The pull request introduces a new rental agreement template within the application framework. It adds a template loader for ApplicationTypes.RENTAL_AGREEMENT, alongside various configuration files, utility functions, and internationalization messages. The changes encompass the creation of forms and sections related to rental housing, landlord and tenant information, rental periods, and associated fees. Additionally, several enums and constants are defined to enhance the application's capability to manage rental agreements effectively.

Changes

File Path Change Summary
libs/application/template-loader/src/lib/templateLoaders.ts Added new entry for ApplicationTypes.RENTAL_AGREEMENT to load the rental agreement template.
libs/application/templates/rental-agreement/.babelrc New Babel configuration file created for the rental agreement template.
libs/application/templates/rental-agreement/.eslintrc.json New ESLint configuration file created for the rental agreement template.
libs/application/templates/rental-agreement/README.md New README file created for the rental agreement template library.
libs/application/templates/rental-agreement/babel-jest.config.json New Jest configuration file for Babel in the rental agreement template.
libs/application/templates/rental-agreement/jest.config.ts New Jest configuration file for the rental agreement template.
libs/application/templates/rental-agreement/project.json New project configuration for the rental agreement template.
libs/application/templates/rental-agreement/src/dataProviders/index.ts Exported new APIs for user profile and national registry.
libs/application/templates/rental-agreement/src/forms/*.ts Multiple new forms and sections created for rental housing information, prerequisites, and agreements.
libs/application/templates/rental-agreement/src/index.ts Introduced module exports for the rental agreement template and related functionalities.
libs/application/templates/rental-agreement/src/lib/*.ts New utility functions, constants, and types introduced for managing rental agreements.
libs/application/types/src/lib/ApplicationTypes.ts Added RENTAL_AGREEMENT to ApplicationTypes enum and corresponding configuration.
libs/application/types/src/lib/InstitutionContentfulIds.ts New entry added to InstitutionContentfulIds enum.
libs/application/types/src/lib/InstitutionMapper.ts New mapping entry for ApplicationTypes.RENTAL_AGREEMENT added.
libs/application/types/src/lib/InstitutionNationalIds.ts New entry added to InstitutionNationalIds enum.
libs/application/types/src/lib/InstitutionTypes.ts New entry added to InstitutionTypes enum.
libs/feature-flags/src/lib/features.ts New feature flag rentalAgreement added.
tsconfig.base.json Added path mapping for rental agreement template.

Possibly related PRs

Suggested labels

automerge

Suggested reviewers

  • fjandakornid
  • Toti91
  • eirikurn
  • FridrikMProgramm

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.

@hebaulf hebaulf closed this Oct 30, 2024
@hebaulf hebaulf reopened this Oct 30, 2024
@hebaulf hebaulf changed the base branch from main to rental-agreement-application October 30, 2024 13:57
@hebaulf hebaulf closed this Oct 30, 2024
@hebaulf hebaulf reopened this Oct 30, 2024
@hebaulf hebaulf requested review from addi and removed request for a team October 30, 2024 13:58
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.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

🛑 Comments failed to post (58)
libs/application/templates/rental-agreement/src/lib/types.ts (1)

11-11: 🛠️ Refactor suggestion

Consider using an enum for StatusProvider

While the union type is valid, using an enum would provide better type safety, maintainability, and IDE support. This is especially important as this type might be used across different parts of the application.

Consider refactoring to:

-export type StatusProvider = 'failure' | 'success'
+export enum StatusProvider {
+  Failure = 'failure',
+  Success = 'success'
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export enum StatusProvider {
  Failure = 'failure',
  Success = 'success'
}
libs/application/templates/rental-agreement/src/index.ts (1)

1-2: 🛠️ Refactor suggestion

Optimize exports to prevent potential circular dependencies.

The current pattern of importing the default export and then re-exporting everything from the same module could lead to circular dependencies and affect tree-shaking.

Consider consolidating the exports:

-import RentalAgreementTemplate from './lib/RentalAgreementTemplate'
-export * from './lib/RentalAgreementTemplate'
+export { default } from './lib/RentalAgreementTemplate'
+export * from './lib/RentalAgreementTemplate'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export { default } from './lib/RentalAgreementTemplate'
export * from './lib/RentalAgreementTemplate'
libs/application/templates/rental-agreement/src/forms/prerequisitesForm.ts (1)

8-15: 🛠️ Refactor suggestion

Add type annotations and documentation for better maintainability.

While the form configuration is functional, it could benefit from improved type safety and documentation.

Consider applying these improvements:

+/**
+ * Prerequisites form for the rental agreement application.
+ * Handles the initial setup and external data collection steps.
+ */
 export const PrerequisitesForm: Form = buildForm({
   id: 'PrerequisitesForm',
   title: m.prerequisites.intro.sectionTitle,
   mode: FormModes.NOT_STARTED,
   renderLastScreenButton: true,
   renderLastScreenBackButton: true,
-  children: [intro, externalData],
+  children: [intro, externalData] as const,
 } as const)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

/**
 * Prerequisites form for the rental agreement application.
 * Handles the initial setup and external data collection steps.
 */
export const PrerequisitesForm: Form = buildForm({
  id: 'PrerequisitesForm',
  title: m.prerequisites.intro.sectionTitle,
  mode: FormModes.NOT_STARTED,
  renderLastScreenButton: true,
  renderLastScreenBackButton: true,
  children: [intro, externalData] as const,
} as const)
libs/application/templates/rental-agreement/src/forms/rentalAgreementForm.ts (1)

1-17: 🛠️ Refactor suggestion

Consider enhancing type safety for form configuration.

To improve reusability and type safety across different NextJS apps (as per coding guidelines), consider:

  1. Defining and exporting interface types for the form data structure
  2. Adding type annotations for the form's state

Add the following type definitions:

export interface RentalAgreementFormData {
  rentalHousingInfo: unknown; // Replace with actual type
  rentalPeriod: unknown; // Replace with actual type
  summary: unknown; // Replace with actual type
}

export const RentalAgreementForm: Form<RentalAgreementFormData> = buildForm({
  // ... existing configuration
})
libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodSecurityDeposit.ts (2)

7-23: ⚠️ Potential issue

Add TypeScript interface for form values

To improve type safety and maintain consistency with coding guidelines, define and export an interface for the form values.

Add this before the form definition:

export interface RentalPeriodSecurityDepositFormData {
  rentalPeriodSecurityDepositInput: string
}

15-19: 🛠️ Refactor suggestion

Enhance security deposit field with validation and structure

The current implementation using a single text field might be insufficient for capturing security deposit details. Consider:

  1. Adding validation rules (e.g., maximum amount, format)
  2. Using more structured fields (amount, payment method, due date)

Consider this enhanced implementation:

buildMultiField({
  id: 'rentalPeriodSecurityDepositDetails',
  title: 'Security Deposit Details',
  children: [
    buildTextField({
      id: 'securityDepositAmount',
      title: 'Amount',
      format: 'number',
      backgroundColor: 'blue',
      required: true,
      validation: [
        {
          type: 'min',
          value: 0,
          message: 'Amount must be positive',
        },
      ],
    }),
    buildDateField({
      id: 'securityDepositDueDate',
      title: 'Due Date',
      required: true,
    }),
    buildSelectField({
      id: 'securityDepositPaymentMethod',
      title: 'Payment Method',
      options: [
        { label: 'Bank Transfer', value: 'bankTransfer' },
        { label: 'Cash', value: 'cash' },
      ],
    }),
  ],
})
libs/application/templates/rental-agreement/src/forms/rentalPeriod.ts (1)

8-17: 🛠️ Refactor suggestion

Consider adding TypeScript interface for section configuration

While the code is functional, adding TypeScript interfaces would improve type safety and documentation. This is especially important for library code that may be reused across different NextJS applications.

Consider adding type definitions:

interface RentalPeriodConfig {
  id: string;
  title: string;
  children: Array<typeof RentalPeriodDetails | typeof RentalPeriodAmount | typeof RentalPeriodSecurityDeposit | typeof RentalPeriodOtherFees>;
}

export const RentalPeriod = buildSection<RentalPeriodConfig>({
  id: 'rentalPeriod',
  title: 'Tímabil og verð',
  children: [
    RentalPeriodDetails,
    RentalPeriodAmount,
    RentalPeriodSecurityDeposit,
    RentalPeriodOtherFees,
  ],
})
libs/application/templates/rental-agreement/src/forms/signing.ts (1)

8-25: 🛠️ Refactor suggestion

Refactor to reduce duplication and add i18n support.

The current implementation has several areas for improvement:

  1. The title "Undirritun" and description are repeated multiple times
  2. Text strings are hardcoded without internationalization support
  3. The nested structure could be simplified as the description field duplicates its parent's content

Consider applying these improvements:

+import { defineMessages } from '@island.is/application/core'
+
+const messages = defineMessages({
+  sectionTitle: {
+    id: 'rental.agreement.signing.title',
+    defaultMessage: 'Undirritun',
+    description: 'Title for the signing section',
+  },
+  sectionDescription: {
+    id: 'rental.agreement.signing.description',
+    defaultMessage: 'Vinsamlegast undirritið samninginn',
+    description: 'Description for the signing section',
+  },
+})

 export const Signing: Section = buildSection({
   id: 'signing',
-  title: 'Undirritun',
+  title: messages.sectionTitle,
   children: [
     buildMultiField({
       id: 'signingInfo',
-      title: 'Undirritun',
-      description: 'Vinsamlegast undirritið samninginn',
+      title: messages.sectionTitle,
+      description: messages.sectionDescription,
-      children: [
-        buildDescriptionField({
-          id: 'signingDescription',
-          title: 'Undirritun',
-          description: 'Vinsamlegast undirritið samninginn',
-        }),
-      ],
     }),
   ],
 })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { defineMessages } from '@island.is/application/core'

const messages = defineMessages({
  sectionTitle: {
    id: 'rental.agreement.signing.title',
    defaultMessage: 'Undirritun',
    description: 'Title for the signing section',
  },
  sectionDescription: {
    id: 'rental.agreement.signing.description',
    defaultMessage: 'Vinsamlegast undirritið samninginn',
    description: 'Description for the signing section',
  },
})

export const Signing: Section = buildSection({
  id: 'signing',
  title: messages.sectionTitle,
  children: [
    buildMultiField({
      id: 'signingInfo',
      title: messages.sectionTitle,
      description: messages.sectionDescription,
    }),
  ],
})
libs/application/templates/rental-agreement/src/forms/prerequisites/intro.ts (1)

7-9: 🛠️ Refactor suggestion

Consider using specific imports for messages

Instead of importing the entire messages namespace, consider importing only the required messages to enable better tree-shaking.

-import * as m from '../../lib/messages'
-const messages = m.prerequisites.intro
+import { prerequisites } from '../../lib/messages'
+const messages = prerequisites.intro
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { prerequisites } from '../../lib/messages'

const messages = prerequisites.intro
libs/application/templates/rental-agreement/src/forms/RentalHousingInfo.ts (2)

10-10: 🛠️ Refactor suggestion

Add type annotation for exported constant.

To improve type safety and documentation, consider adding an explicit type annotation for the exported constant.

- export const RentalHousingInfo = buildSection({
+ export const RentalHousingInfo: Section = buildSection({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export const RentalHousingInfo: Section = buildSection({

1-8: 🛠️ Refactor suggestion

Add TypeScript type definitions for better type safety.

Consider adding type imports for the section configuration and component types to improve type safety and maintainability.

import { buildSection } from '@island.is/application/core'
+ import type { Section } from '@island.is/application/core'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { buildSection } from '@island.is/application/core'
import type { Section } from '@island.is/application/core'

import { RentalHousingPropertyInfo } from './rentalHousingInfo/rentalHousingPropertyInfo'
import { RentalHousingLandlordInfo } from './rentalHousingInfo/rentalHousingLandlordInfo'
import { RentalHousingTenantInfo } from './rentalHousingInfo/rentalHousingTenantInfo'
import { RentalHousingSpecialProvisions } from './rentalHousingInfo/rentalHousingSpecialProvisions'
import { RentalHousingCondition } from './rentalHousingInfo/rentalHousingCondition'
import { RentalHousingFireProtections } from './rentalHousingInfo/rentalHousingFireProtections'
libs/application/templates/rental-agreement/project.json (1)

21-26: 🛠️ Refactor suggestion

Consider adding explicit dependencies for string extraction.

The extract-strings command relies on the localization library's TypeScript configuration and scripts. Consider adding explicit project dependencies to ensure proper build order.

 "extract-strings": {
   "executor": "nx:run-commands",
   "options": {
     "command": "yarn ts-node -P libs/localization/tsconfig.lib.json libs/localization/scripts/extract libs/application/templates/rental-agreement/src/lib/messages.ts"
-  }
+  },
+  "dependsOn": [
+    {
+      "projects": ["localization"],
+      "target": "build"
+    }
+  ]
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    "extract-strings": {
      "executor": "nx:run-commands",
      "options": {
        "command": "yarn ts-node -P libs/localization/tsconfig.lib.json libs/localization/scripts/extract libs/application/templates/rental-agreement/src/lib/messages.ts"
      },
      "dependsOn": [
        {
          "projects": ["localization"],
          "target": "build"
        }
      ]
    }
libs/application/templates/rental-agreement/src/lib/messages/dataSchema.ts (2)

1-3: 🛠️ Refactor suggestion

Add TypeScript types for better type safety.

Consider adding type definitions for the message structure to improve maintainability and type safety.

import { defineMessages } from 'react-intl'
+ import { MessageDescriptor } from 'react-intl'

+ type ValidationMessages = {
+   [K in keyof typeof dataSchema]: MessageDescriptor
+ }

- export const dataSchema = defineMessages({
+ export const dataSchema: ValidationMessages = defineMessages({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { defineMessages } from 'react-intl'
import { MessageDescriptor } from 'react-intl'

type ValidationMessages = {
  [K in keyof typeof dataSchema]: MessageDescriptor
}

export const dataSchema: ValidationMessages = defineMessages({

20-24: 💡 Codebase verification

Based on the search results, I can now provide a conclusive verification. The code shows:

  1. In rentalPeriodOtherFees.ts, the housing fund amount field is configured with maxLength: 13
  2. In dataSchema.ts, there's a validation that enforces a 7-digit limit specifically when housingFund === rentOtherFeesPayeeOptions.TENANT
  3. The field is of type currency variant

Housing fund amount validation is inconsistent

The codebase has conflicting validations for the housing fund amount:

  • The form field allows up to 13 characters
  • The schema validation restricts it to 7 digits when tenant is the payee

This inconsistency should be aligned to prevent confusing user experiences.

🔗 Analysis chain

Verify the housing fund amount constraint.

The 7-digit limit for housing fund amounts should be verified against business requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any other housing fund related validations or constants
rg -g '!*.{log,lock}' -i "housing.?fund|hussjod" -A 5 -B 5

Length of output: 14248

libs/application/templates/rental-agreement/src/forms/prerequisites/externalData.ts (2)

32-43: 🛠️ Refactor suggestion

Consider adding type safety and documentation for data providers

While the data provider configuration is well-structured, consider these improvements:

  1. Add TypeScript interfaces to validate the shape of data from each provider
  2. Include JSDoc comments describing the expected data structure and provider responsibilities

Example implementation:

/** Interface for user profile data */
interface UserProfileData {
  // Add expected fields
}

/** Interface for national registry data */
interface NationalRegistryData {
  // Add expected fields
}

/**
 * User Profile provider fetches current application data
 * @returns UserProfileData
 */

20-31: ⚠️ Potential issue

Use localized messages and improve accessibility

The submit field configuration has several areas for improvement:

  1. The submit button text should use localized messages instead of hardcoded text
  2. The empty title might affect accessibility
  3. The action type should be typed using an enum/const

Consider applying these changes:

 submitField: buildSubmitField({
   id: 'toDraft',
-  title: '',
+  title: m.prerequisites.externalData.submitTitle, // Add this message
   refetchApplicationAfterSubmit: true,
   actions: [
     {
       event: DefaultEvents.SUBMIT,
-      name: 'Hefja umsókn',
+      name: m.prerequisites.externalData.submitButtonText, // Add this message
-      type: 'primary',
+      type: ButtonTypes.PRIMARY, // Add enum import and use it
     },
   ],
 }),

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingSpecialProvisions.ts (2)

9-12: 🛠️ Refactor suggestion

Consider adding explicit type definitions for better maintainability.

While the code is functional, adding explicit TypeScript interfaces for the subsection configuration would improve type safety and documentation.

+interface RentalHousingSpecialProvisionsConfig {
+  id: string;
+  title: string;
+  children: BuildFieldType[];
+}

-export const RentalHousingSpecialProvisions = buildSubSection({
+export const RentalHousingSpecialProvisions = buildSubSection<RentalHousingSpecialProvisionsConfig>({

Committable suggestion was skipped due to low confidence.


32-45: ⚠️ Potential issue

Address inconsistencies in field configurations.

Two issues need attention:

  1. The second text field lacks a character limit while the first one has a 1500 char limit
  2. The marginTop: 6 is a magic number that should be defined as a constant
+const SECTION_MARGIN_TOP = 6;
+const MAX_TEXTAREA_LENGTH = 1500;

 buildTextField({
   id: 'rentalHousingSpecialProvisionsInput',
   title: m.specialProvisions.housingRules.inputLabel,
   placeholder: m.specialProvisions.housingRules.inputPlaceholder,
   variant: 'textarea',
+  maxLength: MAX_TEXTAREA_LENGTH,
   rows: 8,
 }),

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodDetails.ts (1)

18-45: ⚠️ Potential issue

Improve type safety and null handling in the conditional logic.

The current implementation has potential runtime issues:

  1. Missing null checks before accessing includes()
  2. No type validation for the checkbox value
  3. No default value for the checkbox field

Apply these improvements:

 buildDateField({
   id: 'rentalPeriodEndDate',
   title: m.rentalPeriodDetails.endDateTitle,
   placeholder: m.rentalPeriodDetails.endDatePlaceholder,
   condition: (answers) => {
-    const rentalPeriodDefinite: string[] =
-      answers.rentalPeriodDefinite as string[]
-
-    return rentalPeriodDefinite && rentalPeriodDefinite.includes('true')
+    return answers.rentalPeriodDefinite?.includes('true') ?? false
   },
 }),
 buildCheckboxField({
   id: 'rentalPeriodDefinite',
   title: '',
+  defaultValue: [],
   options: [
     {
       value: 'true',
       label: m.rentalPeriodDetails.rentalPeriodDefiniteLabel,
     },
   ],
   spacing: 0,
 }),

Also, consider adding TypeScript types for the form values to ensure type safety:

interface RentalPeriodFormValues {
  rentalPeriodStartDate: string
  rentalPeriodEndDate?: string
  rentalPeriodDefinite: string[]
  rentalPeriodTermination: string
}
🧰 Tools
🪛 Biome

[error] 31-31: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

libs/application/templates/rental-agreement/src/lib/constants.ts (1)

66-72: 🛠️ Refactor suggestion

Improve readability and follow naming conventions.

The function could be improved by:

  1. Using camelCase for the parameter name (days instead of Days)
  2. Adding named constants for time conversions
-export const pruneAfterDays = (Days: number): StateLifeCycle => {
+const MS_PER_DAY = 24 * 60 * 60 * 1000;
+
+export const pruneAfterDays = (days: number): StateLifeCycle => {
   return {
     shouldBeListed: false,
     shouldBePruned: true,
-    whenToPrune: Days * 24 * 3600 * 1000,
+    whenToPrune: days * MS_PER_DAY,
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

const MS_PER_DAY = 24 * 60 * 60 * 1000;

export const pruneAfterDays = (days: number): StateLifeCycle => {
  return {
    shouldBeListed: false,
    shouldBePruned: true,
    whenToPrune: days * MS_PER_DAY,
  }
}
libs/application/templates/rental-agreement/src/lib/messages/housing/tenantDetails.ts (1)

3-50: 🛠️ Refactor suggestion

Consider enhancing type safety and reusability.

The message definitions look good, but we can improve type safety and reusability.

  1. Add explicit type annotation for better type inference:
-export const tenantDetails = defineMessages({
+export const tenantDetails = defineMessages<Record<string, MessageDescriptor>>({
  1. Consider extracting the message prefix to a constant for better maintainability:
const MESSAGE_PREFIX = 'ra.application:tenantDetails' as const

export const tenantDetails = defineMessages<Record<string, MessageDescriptor>>({
  subSectionName: {
    id: `${MESSAGE_PREFIX}.subSectionName`,
    // ...
  },
  // ...
})
libs/application/templates/rental-agreement/src/lib/messages/housing/landlordDetails.ts (2)

1-3: 🛠️ Refactor suggestion

Add TypeScript types for better type safety

Consider adding type imports from 'react-intl' to ensure proper typing of the messages object.

- import { defineMessages } from 'react-intl'
+ import { defineMessages, MessageDescriptor } from 'react-intl'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { defineMessages, MessageDescriptor } from 'react-intl'

export const landlordDetails = defineMessages({

3-50: 🛠️ Refactor suggestion

Consider adding explicit type export

To improve reusability and type safety across different NextJS apps, consider adding an explicit type export for the messages object.

+ export type LandlordDetailsMessages = typeof landlordDetails;
export const landlordDetails = defineMessages({
  // ... existing messages
}) as const;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export type LandlordDetailsMessages = typeof landlordDetails;
export const landlordDetails = defineMessages({
  subSectionName: {
    id: 'ra.application:landlordDetails.subSectionName',
    defaultMessage: 'Leigusali',
    description: 'Landlord details subsection name',
  },
  pageTitle: {
    id: 'ra.application:landlordDetails.pageTitle',
    defaultMessage: 'Skrá leigusala',
    description: 'Landlord details page title',
  },
  pageDescription: {
    id: 'ra.application:landlordDetails.pageDescription',
    defaultMessage:
      'Hér skal skrá leigusala húsnæðis. Hægt er að bæta við eins mörgum leigusölum á samninginn eins og óskað er eftir.',
    description: 'Landlord details page description',
  },
  nationalIdInputLabel: {
    id: 'ra.application:landlordDetails.nationalIdInputLabel',
    defaultMessage: 'Kennitala leigusala',
    description: 'Landlord details national id input label',
  },
  nationalIdHeaderLabel: {
    id: 'ra.application:landlordDetails.nationalIdHeaderLabel',
    defaultMessage: 'Kennitala',
    description: 'Landlord details national id header label',
  },
  nameInputLabel: {
    id: 'ra.application:landlordDetails.nameLabel',
    defaultMessage: 'Fullt nafn',
    description: 'Landlord details name input label',
  },
  emailInputLabel: {
    id: 'ra.application:landlordDetails.emailLabel',
    defaultMessage: 'Netfang',
    description: 'Landlord details email input label',
  },
  phoneInputLabel: {
    id: 'ra.application:landlordDetails.phoneLabel',
    defaultMessage: 'Símanúmer',
    description: 'Landlord details phone input label',
  },
  representativeLabel: {
    id: 'ra.application:landlordDetails.representativeLabel',
    defaultMessage: 'Þessi aðili er umboðsaðili leigusala',
    description: 'Landlord details representative label',
  },
}) as const;
libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingFireProtections.ts (2)

25-31: ⚠️ Potential issue

Add validation constraints for number inputs

The number fields for smoke detectors, fire extinguishers, exits, and fire blankets should have minimum and maximum value constraints to prevent invalid inputs.

 buildTextField({
   id: 'rentalHousingFireProtectionsSmokeDetectors',
   title: m.housingFireProtections.smokeDetectorsLabel,
   placeholder: '0',
   width: 'half',
   variant: 'number',
+  props: {
+    min: 0,
+    max: 999,
+  },
+  validation: {
+    min: {
+      value: 0,
+      message: m.housingFireProtections.invalidNumberMessage,
+    },
+  },
 }),

Apply similar changes to other number fields.

Also applies to: 32-38, 45-51, 52-58


1-8: 🛠️ Refactor suggestion

Add TypeScript types for form builders

Consider adding explicit TypeScript types for the form builders to improve type safety and documentation.

+import { SubSection, TextField, DescriptionField, MultiField } from '@island.is/application/core'
 import {
   buildSubSection,
   buildMultiField,
   buildDescriptionField,
   buildTextField,
 } from '@island.is/application/core'
 import * as m from '../../lib/messages'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { SubSection, TextField, DescriptionField, MultiField } from '@island.is/application/core'
import {
  buildSubSection,
  buildMultiField,
  buildDescriptionField,
  buildTextField,
} from '@island.is/application/core'
import * as m from '../../lib/messages'
libs/application/templates/rental-agreement/src/lib/messages/housing/housingFireProtections.ts (2)

36-41: ⚠️ Potential issue

Fix incorrect description for exitFireBlanketRequirements.

The description field mentions "Smoke detector and fire blanket requirements" but should be "Exit routes and fire blanket requirements" to match the actual content.

   exitFireBlanketRequirements: {
     id: 'ra.application:housingFireProtections.exitFireBlanketRequirements',
     defaultMessage:
       'Flóttaleiðir þurfa að vera auðrataðar og greiðfærar en ekki er gerð krafa um eldvarnarteppi.',
-    description: 'Smoke detector and fire blanket requirements',
+    description: 'Exit routes and fire blanket requirements',
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  exitFireBlanketRequirements: {
    id: 'ra.application:housingFireProtections.exitFireBlanketRequirements',
    defaultMessage:
      'Flóttaleiðir þurfa að vera auðrataðar og greiðfærar en ekki er gerð krafa um eldvarnarteppi.',
    description: 'Exit routes and fire blanket requirements',
  },

1-3: 🛠️ Refactor suggestion

Add TypeScript interface for message definitions.

To improve type safety and maintain consistency with coding guidelines, consider adding TypeScript interfaces for the message definitions.

import { defineMessages } from 'react-intl'

+interface FireProtectionMessages {
+  subSectionName: MessageDescriptor
+  pageTitle: MessageDescriptor
+  pageDescription: MessageDescriptor
+  smokeDetectorsFireExtinguisherRequirements: MessageDescriptor
+  smokeDetectorsLabel: MessageDescriptor
+  fireExtinguisherLabel: MessageDescriptor
+  exitFireBlanketRequirements: MessageDescriptor
+  exitsLabel: MessageDescriptor
+  fireBlanketLabel: MessageDescriptor
+}

-export const housingFireProtections = defineMessages({
+export const housingFireProtections: FireProtectionMessages = defineMessages({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { defineMessages } from 'react-intl'

interface FireProtectionMessages {
  subSectionName: MessageDescriptor
  pageTitle: MessageDescriptor
  pageDescription: MessageDescriptor
  smokeDetectorsFireExtinguisherRequirements: MessageDescriptor
  smokeDetectorsLabel: MessageDescriptor
  fireExtinguisherLabel: MessageDescriptor
  exitFireBlanketRequirements: MessageDescriptor
  exitsLabel: MessageDescriptor
  fireBlanketLabel: MessageDescriptor
}

export const housingFireProtections: FireProtectionMessages = defineMessages({
libs/application/templates/rental-agreement/src/lib/messages/rentalPeriod/rentalPeriodDetails.ts (1)

53-58: 🛠️ Refactor suggestion

Consider type-safety for markdown messages.

The message ID includes a #markdown suffix, indicating markdown content. Consider adding type safety for markdown messages.

Consider creating a type-safe way to handle markdown messages:

type MarkdownMessage = {
  id: string;
  defaultMessage: string;
  description: string;
  markdown: true;
};

// Then use it in your messages:
terminationDescription: {
  id: 'ra.application:rentalPeriodDetails.terminationDescription',
  defaultMessage: '...',
  description: 'Rental period termination description',
  markdown: true,
} as MarkdownMessage
libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingTenantInfo.ts (2)

1-10: 🛠️ Refactor suggestion

Consider adding TypeScript type definitions for better type safety.

To improve type safety and documentation, consider:

  1. Explicitly importing and exporting interface types for the form configuration
  2. Adding type definitions for the expected shape of message objects

Add the following type definitions:

interface TenantInfo {
  name: string;
  nationalId: string;
  phone: string;
  email: string;
  isRepresentative: boolean;
}

export type RentalHousingTenantInfoSubSection = SubSection<TenantInfo>;

20-74: ⚠️ Potential issue

Add validation rules and accessibility improvements.

The table repeater implementation needs the following enhancements:

  1. Add email validation rules
  2. Specify required fields
  3. Set a maximum limit for the number of tenants
  4. Improve checkbox accessibility

Apply these improvements to the configuration:

 buildTableRepeaterField({
   id: 'rentalHousingTenantInfoTable',
   title: '',
   marginTop: 1,
+  maxRows: 10,
   fields: {
     name: {
       component: 'input',
       label: m.tenantDetails.nameInputLabel,
       width: 'half',
+      required: true,
     },
     nationalId: {
       component: 'input',
       label: m.tenantDetails.nationalIdInputLabel,
       format: '######-####',
       width: 'half',
+      required: true,
     },
     email: {
       component: 'input',
       label: m.tenantDetails.emailInputLabel,
       type: 'email',
       width: 'half',
+      required: true,
+      pattern: '^[^@]+@[^@]+\\.[^@]+$',
+      errorMessage: m.tenantDetails.emailFormatError,
     },
     isRepresentative: {
       component: 'checkbox',
       large: true,
       displayInTable: false,
       label: m.tenantDetails.representativeLabel,
+      'aria-label': m.tenantDetails.representativeAriaLabel,
       options: [
         {
           label: m.tenantDetails.representativeLabel,
           value: YES,
         },
       ],
     },
   },

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingLandlordInfo.ts (1)

15-74: 🛠️ Refactor suggestion

Enhance field validation and user experience.

Consider the following improvements:

  1. Add email format validation
  2. Include maximum length constraints for text inputs
  3. Add a tooltip or helper text for the representative checkbox explaining its implications
 email: {
   component: 'input',
   label: m.landlordDetails.emailInputLabel,
   type: 'email',
   width: 'half',
+  pattern: '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$',
+  maxLength: 256,
 },
 name: {
   component: 'input',
   label: m.landlordDetails.nameInputLabel,
   width: 'half',
+  maxLength: 100,
 },
 isRepresentative: {
   component: 'checkbox',
   large: true,
   displayInTable: false,
   label: m.landlordDetails.representativeLabel,
+  tooltip: m.landlordDetails.representativeTooltip,
   options: [

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingCondition.ts (3)

13-75: 🛠️ Refactor suggestion

Add TypeScript interface for form values

To improve type safety and documentation, consider adding an interface that defines the shape of the form values.

Add this interface before the component definition:

interface RentalHousingConditionFormValues {
  rentalHousingConditionInspector: string;
  rentalHousingConditionInspectorName?: string;
  rentalHousingConditionTextInput: string;
  rentalHousingConditionFiles: File[];
}

Then export it for reuse:

+export type { RentalHousingConditionFormValues }
 export const RentalHousingCondition = buildSubSection({

64-64: ⚠️ Potential issue

Replace placeholder ID with descriptive identifier

The ID 'asdf' appears to be a placeholder and should be replaced with a meaningful identifier that reflects the field's purpose, such as 'rentalHousingConditionFiles' or 'conditionInspectionDocuments'.

-          id: 'asdf',
+          id: 'rentalHousingConditionFiles',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

          id: 'rentalHousingConditionFiles',

40-46: 🛠️ Refactor suggestion

Simplify the condition callback

The condition callback can be simplified by directly returning the comparison.

-          condition: (answers) => {
-            const { inspectorOptions } = getApplicationAnswers(answers)
-            return (
-              inspectorOptions ===
-              rentalHousingConditionInspector.INDEPENDENT_PARTY
-            )
-          },
+          condition: (answers) => 
+            getApplicationAnswers(answers).inspectorOptions === 
+            rentalHousingConditionInspector.INDEPENDENT_PARTY,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

          condition: (answers) => 
            getApplicationAnswers(answers).inspectorOptions === 
            rentalHousingConditionInspector.INDEPENDENT_PARTY,
libs/application/templates/rental-agreement/src/lib/messages/prerequisites.ts (1)

1-85: 🛠️ Refactor suggestion

Enhance type safety for message definitions.

While the implementation is solid, consider improving type safety:

// Define a type for the message structure
type PrerequisitesMessages = {
  intro: MessageDescriptor;
  externalData: MessageDescriptor;
};

// Use the type in the export
export const prerequisites: PrerequisitesMessages = {
  // ... existing implementation
};

This will help catch potential structure changes during refactoring and provide better IDE support.

libs/application/templates/rental-agreement/src/lib/messages/housing/housingCondition.ts (2)

1-3: 🛠️ Refactor suggestion

Add TypeScript type definitions for better type safety

Consider adding type definitions for the messages object to improve type safety and maintainability.

import { defineMessages } from 'react-intl'
+ import { MessageDescriptor } from 'react-intl'

+ type HousingConditionMessages = {
+   [K in keyof typeof housingCondition]: MessageDescriptor
+ }

- export const housingCondition = defineMessages({
+ export const housingCondition: HousingConditionMessages = defineMessages({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

import { defineMessages } from 'react-intl'
import { MessageDescriptor } from 'react-intl'

type HousingConditionMessages = {
  [K in keyof typeof housingCondition]: MessageDescriptor
}

export const housingCondition: HousingConditionMessages = defineMessages({

41-50: ⚠️ Potential issue

Fix typo in message IDs: 'independant' should be 'independent'

There's a spelling inconsistency in the message IDs. The word "independant" is misspelled and should be "independent" to maintain consistency with other messages.

- independantInspectorNameLabel: {
+ independentInspectorNameLabel: {
-   id: 'ra.application:housingCondition.independantInspectorNameLabel',
+   id: 'ra.application:housingCondition.independentInspectorNameLabel',
    defaultMessage: 'Fullt nafn',
    description: 'Housing condition independant inspector name',
  },
- independantInspectorNamePlaceholder: {
+ independentInspectorNamePlaceholder: {
-   id: 'ra.application:housingCondition.independantInspectorNamePlaceholder',
+   id: 'ra.application:housingCondition.independentInspectorNamePlaceholder',
    defaultMessage: 'Skrifaðu hér fullt nafn óháðs aðila',
    description: 'Housing condition independant inspector name placeholder',
  },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  independentInspectorNameLabel: {
    id: 'ra.application:housingCondition.independentInspectorNameLabel',
    defaultMessage: 'Fullt nafn',
    description: 'Housing condition independant inspector name',
  },
  independentInspectorNamePlaceholder: {
    id: 'ra.application:housingCondition.independentInspectorNamePlaceholder',
    defaultMessage: 'Skrifaðu hér fullt nafn óháðs aðila',
    description: 'Housing condition independant inspector name placeholder',
  },
libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodAmount.ts (1)

23-30: 🛠️ Refactor suggestion

Consider improving type safety and null handling.

The helper function could be enhanced for better type safety and robustness:

-function rentalAmountConnectedToIndex(answers: FormValue) {
-  const isRentalAmountConnectedToIndex: string[] =
-    answers.isRentalAmountIndexConnected as string[]
-  return (
-    isRentalAmountConnectedToIndex &&
-    isRentalAmountConnectedToIndex.includes(TRUE)
-  )
+function isRentalAmountConnectedToIndex(answers: FormValue): boolean {
+  return answers.isRentalAmountIndexConnected?.includes(TRUE) ?? false
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function isRentalAmountConnectedToIndex(answers: FormValue): boolean {
  return answers.isRentalAmountIndexConnected?.includes(TRUE) ?? false
}
🧰 Tools
🪛 Biome

[error] 27-28: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

libs/application/templates/rental-agreement/src/lib/messages/rentalInfo/rentalAmount.ts (1)

116-126: 🛠️ Refactor suggestion

Consider adding more insurance-related messages

The insurance section might benefit from additional messages for:

  • Insurance amount
  • Types of accepted insurance
  • Insurance provider details

Example additions:

  paymentInsuranceAmount: {
    id: 'ra.application:rentalAmount.paymentInsuranceAmount',
    defaultMessage: 'Upphæð tryggingar',
    description: 'Amount required for rental insurance',
  },
  paymentInsuranceTypes: {
    id: 'ra.application:rentalAmount.paymentInsuranceTypes',
    defaultMessage: 'Tegundir trygginga',
    description: 'Types of accepted insurance for the rental',
  },
libs/application/templates/rental-agreement/src/lib/utils.ts (3)

20-75: 🛠️ Refactor suggestion

Consider refactoring for better maintainability.

The function follows a repetitive pattern that could be simplified using a configuration-driven approach.

Consider refactoring to reduce repetition:

interface AnswerConfig {
  path: string
  type: string
}

const answerConfigs: Record<string, AnswerConfig> = {
  propertyCategoryTypeOptions: {
    path: 'registerPropertyCategoryType',
    type: 'rentalHousingCategoryTypes'
  },
  // ... other configs
}

export const getApplicationAnswers = (answers: Application['answers']) => {
  return Object.entries(answerConfigs).reduce((acc, [key, config]) => ({
    ...acc,
    [key]: getValueViaPath(answers, config.path)
  }), {})
}

77-185: 🛠️ Refactor suggestion

Optimize options generation functions.

Since these functions return static arrays, they could benefit from memoization and TypeScript const assertions to prevent unnecessary re-renders and improve type inference.

Consider using useMemo when used in React components and const assertions for better type inference:

import { useMemo } from 'react'

export const getPropertyCategoryTypeOptions = () => {
  return useMemo(() => [{
    value: rentalHousingCategoryTypes.ENTIRE_HOME,
    label: m.registerProperty.category.typeSelectLabelEntireHome,
  }] as const, [])
}

Also consider extracting the options arrays to constants:

export const PROPERTY_CATEGORY_TYPE_OPTIONS = [{
  value: rentalHousingCategoryTypes.ENTIRE_HOME,
  label: m.registerProperty.category.typeSelectLabelEntireHome,
}] as const

export const getPropertyCategoryTypeOptions = () => PROPERTY_CATEGORY_TYPE_OPTIONS

17-18: ⚠️ Potential issue

Handle edge cases in formatNationalId function.

The function might return '-' for empty strings, but it doesn't handle other invalid inputs like strings shorter than 6 characters.

Consider adding input validation:

 export const formatNationalId = (nationalId: string) =>
-  insertAt(nationalId.replace('-', ''), '-', 6) || '-'
+  !nationalId ? '-' : nationalId.length < 6 ? nationalId : insertAt(nationalId.replace('-', ''), '-', 6)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export const formatNationalId = (nationalId: string) =>
  !nationalId ? '-' : nationalId.length < 6 ? nationalId : insertAt(nationalId.replace('-', ''), '-', 6)
libs/application/templates/rental-agreement/src/lib/messages/rentalInfo/otherFees.ts (1)

45-81: 🛠️ Refactor suggestion

Consider consolidating duplicate meter-related messages.

The electricity and heating cost sections contain identical message structures with different prefixes. Consider refactoring to use a shared message template to improve maintainability.

Example approach:

const createMeterMessages = (type: 'electricity' | 'heating') => ({
  [`${type}CostMeterNumberLabel`]: {
    id: `ra.application:otherFees.${type}CostMeterNumberLabel`,
    defaultMessage: `Númer ${type === 'electricity' ? 'rafmagns' : 'hita'}mælis`,
    description: `${type} cost meter number label`,
  },
  // ... other meter-related messages
});

export const otherFees = defineMessages({
  ...createMeterMessages('electricity'),
  ...createMeterMessages('heating'),
  // ... other messages
});

Also applies to: 82-117

libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodOtherFees.ts (3)

32-39: 🛠️ Refactor suggestion

Enhance type safety and null checking in otherCostsPayedByTenant.

The function could benefit from improved null checking and type safety.

 function otherCostsPayedByTenant(answers: FormValue) {
   const rentOtherFeesOtherCosts = getValueViaPath(
     answers,
     'rentOtherFees.otherCosts',
     [],
   ) as string[]
-  return rentOtherFeesOtherCosts && rentOtherFeesOtherCosts.includes(TRUE)
+  return rentOtherFeesOtherCosts?.includes(TRUE) ?? false
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function otherCostsPayedByTenant(answers: FormValue) {
  const rentOtherFeesOtherCosts = getValueViaPath(
    answers,
    'rentOtherFees.otherCosts',
    [],
  ) as string[]
  return rentOtherFeesOtherCosts?.includes(TRUE) ?? false
}
🧰 Tools
🪛 Biome

[error] 38-38: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


41-166: 🛠️ Refactor suggestion

Enhance accessibility with ARIA labels.

Consider adding aria-labels to improve screen reader support for the form fields.

Example implementation for the housing fund section:

 buildRadioField({
   id: 'rentOtherFees.housingFund',
   title: m.otherFees.housingFundTitle,
   options: getRentalOtherFeesPayeeOptions,
   width: 'half',
   space: 1,
+  ariaLabel: m.otherFees.housingFundAriaLabel,
 }),

Please add similar aria-labels for other form fields as well.

Committable suggestion was skipped due to low confidence.


57-64: ⚠️ Potential issue

Add validation constraints to the housing fund amount field.

The currency field lacks proper validation constraints for minimum and maximum values.

 buildTextField({
   id: 'rentOtherFees.housingFundAmount',
   title: m.otherFees.housingFundAmountLabel,
   placeholder: m.otherFees.housingFundAmountPlaceholder,
   variant: 'currency',
   maxLength: 13,
+  validation: {
+    min: 0,
+    max: 999999999,
+    required: true,
+  },
   condition: housingFundAmountPayedByTenant,
 }),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        buildTextField({
          id: 'rentOtherFees.housingFundAmount',
          title: m.otherFees.housingFundAmountLabel,
          placeholder: m.otherFees.housingFundAmountPlaceholder,
          variant: 'currency',
          maxLength: 13,
          validation: {
            min: 0,
            max: 999999999,
            required: true,
          },
          condition: housingFundAmountPayedByTenant,
        }),
libs/application/template-loader/src/lib/templateLoaders.ts (1)

208-209: 💡 Codebase verification

Import path configuration needs adjustment

The template directory exists and contains the necessary index files, but there's a mismatch in the path configuration:

  • The tsconfig path mapping is @island.is/application/templates/rental-agreement (without /*)
  • The import statement uses the same path, which is correct
  • However, the package.json is missing in the template directory, which is required for proper module resolution

Required actions:

  • Add package.json to the rental-agreement template directory
  • Ensure the path mapping in tsconfig.base.json includes the /* suffix for proper module resolution
🔗 Analysis chain

LGTM! Verify import path configuration.

The implementation follows the established patterns for template loaders and maintains consistency with existing entries.

Let's verify the import path configuration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the rental-agreement template path is properly configured

# Test 1: Check if the path is mapped in tsconfig
echo "Checking tsconfig.base.json for path mapping..."
cat tsconfig.base.json | jq -r '.compilerOptions.paths["@island.is/application/templates/rental-agreement/*"]'

# Test 2: Verify the template directory exists
echo "Checking if template directory exists..."
fd -t d "rental-agreement$" -p libs/application/templates/

Length of output: 419


Script:

#!/bin/bash
# Let's verify the path mapping in tsconfig and package existence

# Check the exact path mapping pattern in tsconfig
echo "Checking all path mappings in tsconfig.base.json..."
cat tsconfig.base.json | jq -r '.compilerOptions.paths | keys[]' | grep -i rental-agreement

# Check if the template exists as a package
echo "Checking package.json in the template directory..."
cat libs/application/templates/rental-agreement/package.json 2>/dev/null || echo "package.json not found"

# Check for index files
echo "Checking for index files..."
fd -t f "^index\.(ts|js)$" libs/application/templates/rental-agreement/

# Check template registration
echo "Checking template registration..."
rg -A 2 "RENTAL_AGREEMENT" libs/application/template-loader/src/lib/

Length of output: 1260

libs/application/templates/rental-agreement/src/lib/messages/housing/registerProperty.ts (3)

12-84: 🛠️ Refactor suggestion

Consider extracting common form field messages.

Several messages (e.g., address, postal code, municipality) are generic enough to be reused across different forms. Consider:

  1. Moving common field messages to a shared location
  2. Making the external link URL in pageDescription configurable

Example approach:

// libs/application/shared/messages/formFields.ts
export const commonFormFields = defineMessages({
  address: {
    label: { /* ... */ },
    placeholder: { /* ... */ }
  },
  postalCode: { /* ... */ },
  municipality: { /* ... */ }
})

// Current file
import { commonFormFields } from '@island.is/application/shared/messages'

125-232: 🛠️ Refactor suggestion

Consider making property types and classifications configuration-driven.

The current implementation hardcodes all property types and classifications. Consider:

  1. Moving these options to a configuration file
  2. Generating messages dynamically based on the configuration

This would make it easier to:

  • Add/remove options without code changes
  • Maintain consistency across the application
  • Support different property types per region/country

Example approach:

// libs/application/templates/rental-agreement/src/lib/config/propertyTypes.ts
export const propertyTypes = [
  { id: 'entireHome', defaultMessage: 'Íbúð / Einbýlishús' },
  { id: 'room', defaultMessage: 'Herbergi' },
  // ...
] as const

// Current file
import { propertyTypes } from '../config/propertyTypes'

export const registerProperty = {
  // ...
  category: {
    ...defineMessages({
      // ... static messages ...
    }),
    types: propertyTypes.reduce((acc, type) => ({
      ...acc,
      [`typeSelectLabel${type.id}`]: {
        id: `ra.application:registerProperty.category.typeSelectLabel${type.id}`,
        defaultMessage: type.defaultMessage,
        description: `Label for ${type.id} select option`,
      },
    }), {}),
  },
}

1-3: 🛠️ Refactor suggestion

Add TypeScript types for better type safety and documentation.

Consider adding TypeScript types to improve type safety and documentation. This will help with reusability across different NextJS apps.

import { defineMessages } from 'react-intl'

+type RegisterPropertyMessages = {
+  subsection: Record<string, MessageDescriptor>
+  info: Record<string, MessageDescriptor>
+  infoSummary: Record<string, MessageDescriptor>
+  category: Record<string, MessageDescriptor>
+}
+
-export const registerProperty = {
+export const registerProperty: RegisterPropertyMessages = {

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/lib/RentalAgreementTemplate.ts (2)

84-87: ⚠️ Potential issue

Inconsistent state target reference in state machine configuration

In the on property of the States.DRAFT state, the target is set as the string 'draft'. For consistency and to avoid potential typos or errors, it is recommended to use the state constant States.DRAFT instead.

Apply this diff to fix the inconsistency:

  on: {
    [DefaultEvents.SUBMIT]: {
-     target: 'draft',
+     target: States.DRAFT,
    },
  },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

          [DefaultEvents.SUBMIT]: {
            target: States.DRAFT,
          },
        },

70-79: ⚠️ Potential issue

Missing actions for the applicant role in the DRAFT state

The roles definition for the APPLICANT in the States.DRAFT state lacks the actions property. Without defined actions, the frontend may not display the necessary controls for the applicant to proceed.

Consider adding the appropriate actions to enable users to submit or navigate the application in the DRAFT state. For example:

roles: [
  {
    id: Roles.APPLICANT,
    formLoader: () =>
      import('../forms/rentalAgreementForm').then((module) =>
        Promise.resolve(module.RentalAgreementForm),
      ),
+   actions: [
+     { event: 'SUBMIT', name: 'Submit', type: 'primary' },
+   ],
    write: 'all',
    read: 'all',
    delete: true,
  },
],

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/lib/dataSchema.ts (3)

39-44: ⚠️ Potential issue

Replace 'params' with 'message' in validation error handling

In your ctx.addIssue calls and refine methods, you should use the message property instead of params to specify custom error messages. The params property is not used by Zod for error messages.

Apply this diff to correct the error handling:

-            params: m.dataSchema.errorHousingFundLength,
+            message: m.dataSchema.errorHousingFundLength,

Repeat this replacement for all instances where params is used in ctx.addIssue and refine methods.

Also applies to: 52-57, 63-68, 76-81, 87-92, 104-105


98-109: 🛠️ Refactor suggestion

Ensure TypeScript best practices and reusability

To enhance type safety and reusability across different NextJS apps, consider exporting types derived from your Zod schemas.

For example, you can export the inferred types:

export type DataSchema = z.infer<typeof dataSchema>;
export type RentOtherFees = z.infer<typeof rentOtherFees>;

107-107: ⚠️ Potential issue

Replace placeholder 'asdf' with a meaningful field name

The field asdf appears to be a placeholder. Please replace it with a descriptive name that reflects its purpose in the schema.

For example:

-  asdf: z.array(fileSchema),
+  attachments: z.array(fileSchema),

Committable suggestion was skipped due to low confidence.

libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingPropertyInfo.ts (3)

37-93: 🛠️ Refactor suggestion

Enhance reusability by abstracting repeated field definitions

Multiple fields with similar configurations are defined individually. To improve reusability and maintainability, consider creating reusable components or helper functions for these form fields.


167-174: ⚠️ Potential issue

Correct the variable used in the condition

In the condition function for registerPropertyCategoryClassGroup, the variable propertyCategoryClassOptions seems incorrect. It should likely be propertyCategoryClass to match the field ID defined earlier.

Apply this diff to fix the variable:

 const { propertyCategoryClass } =
   getApplicationAnswers(answers)
 return (
-  propertyCategoryClassOptions ===
+  propertyCategoryClass ===
   rentalHousingCategoryClass.SPECIAL_GROUPS
 )

Ensure that propertyCategoryClass corresponds to the selected value from the registerPropertyCategoryClass field.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

          condition: (answers) => {
            const { propertyCategoryClass } =
              getApplicationAnswers(answers)
            return (
              propertyCategoryClass ===
              rentalHousingCategoryClass.SPECIAL_GROUPS
            )
          },

99-99: ⚠️ Potential issue

Use a safer condition check for address validation

The condition answers.registerPropertyInfoAddress !== '' may not handle undefined or null values properly. Consider using a more robust check to ensure the component behaves as expected.

Apply this diff to improve the condition:

-condition: (answers) => answers.registerPropertyInfoAddress !== '',
+condition: (answers) => !!answers.registerPropertyInfoAddress,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      condition: (answers) => !!answers.registerPropertyInfoAddress,

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.67%. Comparing base (e1d99a8) to head (433fcf2).
Report is 1 commits behind head on rental-agreement-application.

Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                        @@
##           rental-agreement-application   #16650      +/-   ##
================================================================
- Coverage                         36.67%   36.67%   -0.01%     
================================================================
  Files                              6859     6858       -1     
  Lines                            142897   142943      +46     
  Branches                          40822    40909      +87     
================================================================
+ Hits                              52412    52419       +7     
- Misses                            90485    90524      +39     
Flag Coverage Δ
api 3.37% <ø> (ø)
application-system-api 41.32% <ø> (ø)
application-ui-shell 21.22% <ø> (ø)

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

see 47 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 e1d99a8...433fcf2. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Oct 30, 2024

Datadog Report

All test runs 5e96184 🔗

3 Total Test Services: 0 Failed, 3 Passed
➡️ Test Sessions change in coverage: 6 no change

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
api 0 0 0 4 0 2.94s 1 no change Link
application-system-api 0 0 0 120 2 3m 19.01s 1 no change Link
application-ui-shell 0 0 0 74 0 30.81s 1 no change Link

@hebaulf hebaulf merged commit fbd631d into rental-agreement-application Oct 31, 2024
36 checks passed
@hebaulf hebaulf deleted the feat/other-fees-updated branch October 31, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants