feat(rental-agreement): security deposit #16661
Conversation
WalkthroughThe changes in this pull request introduce a new rental agreement application type within the template loader, along with several configuration files, forms, and internationalization messages specific to the rental agreement. A new module structure is created, including types, utility functions, and feature flags to support the new application type. The updates enhance the modularity and extensibility of the application, allowing for better management of rental agreement data and user interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RentalAgreementTemplate
participant FormComponents
participant API
User->>RentalAgreementTemplate: Start Rental Agreement
RentalAgreementTemplate->>FormComponents: Load Prerequisites Form
FormComponents->>User: Display Prerequisites
User->>FormComponents: Fill in Data
FormComponents->>API: Submit Data
API->>RentalAgreementTemplate: Process Data
RentalAgreementTemplate->>User: Display Confirmation
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (61)
libs/application/templates/rental-agreement/src/lib/types.ts (1)
3-9: 🛠️ Refactor suggestion
Consider enhancing type safety and documentation
- The
datefield could benefit from a more specific type likeISO8601StringorDate- Consider adding JSDoc comments to document the interface's purpose and field usage
+/** + * External data structure for rental agreement applications + * Contains national registry information with status tracking + */ export interface ExternalData { nationalRegistry: { data: NationalRegistryIndividual - date: string + date: ISO8601String // Consider creating a type alias for ISO8601String = string status: StatusProvider } }Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/lib/messages/dataSchema.ts (1)
1-14: 🛠️ Refactor suggestion
Consider enhancing type safety with TypeScript.
The implementation looks good but could benefit from explicit TypeScript types for better maintainability and type safety.
Consider applying this enhancement:
import { defineMessages } from 'react-intl' + +type DataSchemaMessages = { + nationalId: { id: string; defaultMessage: string; description: string } + phoneNumber: { id: string; defaultMessage: string; description: string } +} -export const dataSchema = defineMessages({ +export const dataSchema = defineMessages<DataSchemaMessages>({ nationalId: {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { defineMessages } from 'react-intl' type DataSchemaMessages = { nationalId: { id: string; defaultMessage: string; description: string } phoneNumber: { id: string; defaultMessage: string; description: string } } export const dataSchema = defineMessages<DataSchemaMessages>({ nationalId: { id: 'ra.application:dataSchema.national.id', defaultMessage: 'Kennitala er ekki á réttu formi', description: 'Error message when nationalid is wrong', }, phoneNumber: { id: 'ra.application:dataSchema.phoneNumber', defaultMessage: 'Símanúmerið þarf að vera gilt.', description: 'Error message when phone number is invalid.', }, })libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodOtherFees.ts (1)
7-23: 🛠️ Refactor suggestion
Consider enhancing the form structure and accessibility.
- The same title "Önnur gjöld" is repeated at three levels (subsection, multi-field, and text field), which might be confusing for users.
- A text field might not be the most appropriate format for fee input. Consider using a number field with currency validation.
- Missing helper text or description to guide users on what types of fees should be entered.
Consider applying these improvements:
export const RentalPeriodOtherFees = buildSubSection({ id: 'rentalPeriodOtherFees', title: 'Önnur gjöld', children: [ buildMultiField({ id: 'rentalPeriodOtherFeesDetails', - title: 'Önnur gjöld', + title: 'Upplýsingar um önnur gjöld', // "Information about other fees" children: [ buildTextField({ id: 'rentalPeriodOtherFeesInput', - title: 'Önnur gjöld', - format: 'text', + title: 'Upphæð', // "Amount" + format: 'number', + description: 'Vinsamlegast tilgreinið upphæð annarra gjalda ef einhver eru', // "Please specify the amount of other fees if any" + placeholder: '0', + variant: 'currency', }), ], }), ], })📝 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 RentalPeriodOtherFees = buildSubSection({ id: 'rentalPeriodOtherFees', title: 'Önnur gjöld', children: [ buildMultiField({ id: 'rentalPeriodOtherFeesDetails', title: 'Upplýsingar um önnur gjöld', children: [ buildTextField({ id: 'rentalPeriodOtherFeesInput', title: 'Upphæð', format: 'number', description: 'Vinsamlegast tilgreinið upphæð annarra gjalda ef einhver eru', placeholder: '0', variant: 'currency', }), ], }), ], })libs/application/templates/rental-agreement/src/forms/rentalPeriod.ts (2)
9-10: 🛠️ Refactor suggestion
Consider extracting the title to a translation file for better maintainability.
The hardcoded Icelandic title 'Tímabil og verð' should be moved to a translation file to support internationalization and improve reusability across different locales.
Consider applying this enhancement:
+import { useTranslations } from '@island.is/application/core' export const RentalPeriod = buildSection({ id: 'rentalPeriod', - title: 'Tímabil og verð', + title: useTranslations('rental.agreement.period.title'), children: [ // ... ], })Committable suggestion was skipped due to low confidence.
8-17: 🛠️ Refactor suggestion
Consider adding TypeScript type annotations for better type safety.
The section configuration could benefit from explicit type annotations to ensure type safety and improve maintainability.
Consider applying this enhancement:
+import { BuildSectionProps } from '@island.is/application/core' -export const RentalPeriod = buildSection({ +export const RentalPeriod = buildSection<BuildSectionProps>({ id: 'rentalPeriod', title: 'Tímabil og verð', children: [ RentalPeriodDetails, RentalPeriodAmount, RentalPeriodSecurityDeposit, RentalPeriodOtherFees, ], })Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/signing.ts (1)
8-25: 🛠️ Refactor suggestion
Extract repeated strings and add i18n support.
The same title and description are duplicated across different levels. Consider:
- Using i18n keys instead of hardcoded strings
- Extracting common strings to constants
Here's a suggested refactor:
+import { defineMessages } from '@island.is/application/core' + +const messages = defineMessages({ + sectionTitle: { + id: 'rental.agreement.signing.title', + defaultMessage: 'Undirritun', + }, + signDescription: { + id: 'rental.agreement.signing.description', + defaultMessage: 'Vinsamlegast undirritið samninginn', + }, +}) 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.signDescription, children: [ buildDescriptionField({ id: 'signingDescription', - title: 'Undirritun', - description: 'Vinsamlegast undirritið samninginn', + title: messages.sectionTitle, + description: messages.signDescription, }), ], }), ], })📝 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', }, signDescription: { id: 'rental.agreement.signing.description', defaultMessage: 'Vinsamlegast undirritið samninginn', }, }) export const Signing: Section = buildSection({ id: 'signing', title: messages.sectionTitle, children: [ buildMultiField({ id: 'signingInfo', title: messages.sectionTitle, description: messages.signDescription, children: [ buildDescriptionField({ id: 'signingDescription', title: messages.sectionTitle, description: messages.signDescription, }), ], }), ], })libs/application/templates/rental-agreement/src/forms/prerequisites/intro.ts (2)
9-9: 🛠️ Refactor suggestion
Update messages reference.
If implementing the specific import suggestion above, update this line accordingly.
-const messages = m.prerequisites.intro +const messages = prerequisites.introCommittable suggestion was skipped due to low confidence.
7-7: 🛠️ Refactor suggestion
Consider using specific imports for messages.
Instead of importing the entire messages namespace, consider importing only the required prerequisites.intro subset for better tree-shaking.
-import * as m from '../../lib/messages' +import { prerequisites } from '../../lib/messages'Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/RentalHousingInfo.ts (1)
10-21: 🛠️ Refactor suggestion
Enhance type safety and internationalization.
Two suggestions for improvement:
- Add type annotations to the configuration object for better type safety and documentation.
- Consider extracting the title to a translation file for better maintainability and reusability across different locales.
Here's how you could improve the code:
-export const RentalHousingInfo = buildSection({ +export const RentalHousingInfo = buildSection<BuildSectionProps>({ id: 'rentalHousingInfo', - title: 'Húsnæðið', + title: m.rentalHousingInfo.title, children: [ RentalHousingPropertyInfo, RentalHousingLandlordInfo, RentalHousingTenantInfo, RentalHousingSpecialProvisions, RentalHousingCondition, RentalHousingFireProtections, ], })Don't forget to:
- Import the messages:
import { m } from '../messages'- Add the translation key in your messages file
Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/prerequisites/externalData.ts (1)
20-31:
⚠️ Potential issueInternationalize submit field text.
The submit field configuration contains hardcoded text that should be internationalized:
- Empty title string
- Hardcoded Icelandic action name "Hefja umsókn"
Apply this diff to use i18n messages:
submitField: buildSubmitField({ id: 'toDraft', - title: '', + title: m.prerequisites.externalData.submitTitle, refetchApplicationAfterSubmit: true, actions: [ { event: DefaultEvents.SUBMIT, - name: 'Hefja umsókn', + name: m.prerequisites.externalData.submitActionName, type: 'primary', }, ], }),Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/lib/constants.ts (2)
20-39: 🛠️ Refactor suggestion
Consider grouping related enums under a namespace.
The housing category enums are closely related and could benefit from being grouped under a namespace for better organization and type safety.
+export namespace RentalHousing { -export enum rentalHousingCategoryTypes { + export enum CategoryTypes { ENTIRE_HOME = 'entireHome', ROOM = 'room', COMMERCIAL = 'commercial', } -export enum rentalHousingCategoryClass { + export enum CategoryClass { GENERAL_MARKET = 'generalMarket', SPECIAL_GROUPS = 'specialGroups', } -export enum rentalHousingCategoryClassGroup { + export enum CategoryClassGroup { STUDENT_HOUSING = 'studentHousing', SENIOR_CITIZEN_HOUSING = 'seniorCitizenHousing', COMMUNE = 'commune', HALFWAY_HOUSE = 'halfwayHouse', SOCIAL_HOUSING = 'socialHousing', INCOME_BASED_HOUSING = 'incomeBasedHousing', EMPLOYEE_HOUSING = 'employeeHousing', } +}📝 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 namespace RentalHousing { export enum CategoryTypes { ENTIRE_HOME = 'entireHome', ROOM = 'room', COMMERCIAL = 'commercial', } export enum CategoryClass { GENERAL_MARKET = 'generalMarket', SPECIAL_GROUPS = 'specialGroups', } export enum CategoryClassGroup { STUDENT_HOUSING = 'studentHousing', SENIOR_CITIZEN_HOUSING = 'seniorCitizenHousing', COMMUNE = 'commune', HALFWAY_HOUSE = 'halfwayHouse', SOCIAL_HOUSING = 'socialHousing', INCOME_BASED_HOUSING = 'incomeBasedHousing', EMPLOYEE_HOUSING = 'employeeHousing', } }
58-64: 🛠️ Refactor suggestion
Improve maintainability of time conversion logic.
The function could benefit from the following improvements:
- Use camelCase for parameter naming (
daysinstead ofDays)- Extract magic numbers into named constants
+const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000; + -export const pruneAfterDays = (Days: number): StateLifeCycle => { +export const pruneAfterDays = (days: number): StateLifeCycle => { return { shouldBeListed: false, shouldBePruned: true, - whenToPrune: Days * 24 * 3600 * 1000, + whenToPrune: days * MILLISECONDS_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 MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000; export const pruneAfterDays = (days: number): StateLifeCycle => { return { shouldBeListed: false, shouldBePruned: true, whenToPrune: days * MILLISECONDS_PER_DAY, } }libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingSpecialProvisions.ts (2)
1-8: 🛠️ Refactor suggestion
Consider exporting field types for reuse.
To improve reusability across different NextJS apps, consider exporting the field configuration types.
Add the following type definitions:
import { SubSection, TextField } from '@island.is/application/core' export type RentalHousingSpecialProvisionsField = TextField & { maxLength?: number rows?: number } export type RentalHousingSpecialProvisionsSection = SubSection
7-7: 🛠️ Refactor suggestion
Consider using named imports for better tree-shaking.
Replace the namespace import with specific named imports to enable better tree-shaking.
-import * as m from '../../lib/messages' +import { specialProvisions } from '../../lib/messages'Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodDetails.ts (3)
1-9: 🛠️ Refactor suggestion
Consider destructuring specific messages.
Instead of importing all messages with namespace import, consider destructuring only the required messages for better tree-shaking.
-import * as m from '../../lib/messages' +import { rentalPeriodDetails } from '../../lib/messages'Committable suggestion was skipped due to low confidence.
34-44:
⚠️ Potential issueAdd title for better accessibility.
The checkbox field has an empty title which might affect screen readers.
buildCheckboxField({ id: 'rentalPeriodDefinite', - title: '', + title: m.rentalPeriodDetails.rentalPeriodDefiniteTitle, options: [ { value: 'true', label: m.rentalPeriodDetails.rentalPeriodDefiniteLabel, }, ], spacing: 0, }),Committable suggestion was skipped due to low confidence.
18-33:
⚠️ Potential issueAdd date validation and improve type safety.
The date fields lack validation rules, and the condition logic could be improved:
- Consider adding validation rules for dates
- The type assertion could be replaced with proper typing
buildDateField({ id: 'rentalPeriodStartDate', title: m.rentalPeriodDetails.startDateTitle, placeholder: m.rentalPeriodDetails.startDatePlaceholder, + validation: [ + { + id: 'futureDate', + condition: (value) => new Date(value) > new Date(), + errorMessage: m.rentalPeriodDetails.startDateFutureError, + }, + ], }),Also, improve the condition logic:
- condition: (answers) => { - const rentalPeriodDefinite: string[] = - answers.rentalPeriodDefinite as string[] - - return rentalPeriodDefinite && rentalPeriodDefinite.includes('true') - }, + condition: (answers) => + answers.rentalPeriodDefinite?.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.buildDateField({ id: 'rentalPeriodStartDate', title: m.rentalPeriodDetails.startDateTitle, placeholder: m.rentalPeriodDetails.startDatePlaceholder, }), buildDateField({ id: 'rentalPeriodEndDate', title: m.rentalPeriodDetails.endDateTitle, placeholder: m.rentalPeriodDetails.endDatePlaceholder, condition: (answers) => answers.rentalPeriodDefinite?.includes('true') ?? false, }),🧰 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/messages/housing/tenantDetails.ts (2)
1-3: 🛠️ Refactor suggestion
Consider adding TypeScript type definitions for better type safety.
While the code is functional, adding explicit types would improve maintainability and type safety.
Consider applying this improvement:
import { defineMessages } from 'react-intl' + type TenantDetailsMessages = { + subSectionName: { id: string; defaultMessage: string; description: string } + pageTitle: { id: string; defaultMessage: string; description: string } + pageDescription: { id: string; defaultMessage: string; description: string } + nationalIdInputLabel: { id: string; defaultMessage: string; description: string } + nationalIdHeaderLabel: { id: string; defaultMessage: string; description: string } + nameInputLabel: { id: string; defaultMessage: string; description: string } + emailInputLabel: { id: string; defaultMessage: string; description: string } + phoneInputLabel: { id: string; defaultMessage: string; description: string } + representativeLabel: { id: string; defaultMessage: string; description: string } + } - export const tenantDetails = defineMessages({ + export const tenantDetails = defineMessages<TenantDetailsMessages>({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { defineMessages } from 'react-intl' type TenantDetailsMessages = { subSectionName: { id: string; defaultMessage: string; description: string } pageTitle: { id: string; defaultMessage: string; description: string } pageDescription: { id: string; defaultMessage: string; description: string } nationalIdInputLabel: { id: string; defaultMessage: string; description: string } nationalIdHeaderLabel: { id: string; defaultMessage: string; description: string } nameInputLabel: { id: string; defaultMessage: string; description: string } emailInputLabel: { id: string; defaultMessage: string; description: string } phoneInputLabel: { id: string; defaultMessage: string; description: string } representativeLabel: { id: string; defaultMessage: string; description: string } } export const tenantDetails = defineMessages<TenantDetailsMessages>({
4-49: 🛠️ Refactor suggestion
Consider adding validation and error messages for form fields.
The messages cover the basic UI elements but lack validation-related messages that users might encounter when filling out the form.
Consider adding these additional messages:
export const tenantDetails = defineMessages({ // ... existing messages ... + nationalIdInvalidFormat: { + id: 'ra.application:tenantDetails.nationalIdInvalidFormat', + defaultMessage: 'Kennitala er ekki á réttu formi', + description: 'Error message when national ID format is invalid', + }, + emailInvalidFormat: { + id: 'ra.application:tenantDetails.emailInvalidFormat', + defaultMessage: 'Netfang er ekki á réttu formi', + description: 'Error message when email format is invalid', + }, + phoneInvalidFormat: { + id: 'ra.application:tenantDetails.phoneInvalidFormat', + defaultMessage: 'Símanúmer er ekki á réttu formi', + description: 'Error message when phone number format is invalid', + }, })📝 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.subSectionName: { id: 'ra.application:tenantDetails.subSectionName', defaultMessage: 'Leigjandi', description: 'Tenant Details sub section name', }, pageTitle: { id: 'ra.application:tenantDetails.pageTitle', defaultMessage: 'Skrá leigjanda', description: 'Tenant Details page title', }, pageDescription: { id: 'ra.application:tenantDetails.pageDescription', defaultMessage: 'Hér skal skrá leigjendur í húsnæðinu. Hægt er að bæta við eins mörgum leigjendum á samninginn eins og óskað er eftir.', description: 'Tenant Details page description', }, nationalIdInputLabel: { id: 'ra.application:tenantDetails.nationalIdLabel', defaultMessage: 'Kennitala leigjanda', description: 'Tenant Details national id input label', }, nationalIdHeaderLabel: { id: 'ra.application:tenantDetails.nationalIdHeaderLabel', defaultMessage: 'Kennitala', description: 'Tenant details national id header label', }, nameInputLabel: { id: 'ra.application:tenantDetails.nameLabel', defaultMessage: 'Fullt nafn', description: 'Tenant Details name input label', }, emailInputLabel: { id: 'ra.application:tenantDetails.emailLabel', defaultMessage: 'Netfang', description: 'Tenant Details email input label', }, phoneInputLabel: { id: 'ra.application:tenantDetails.phoneLabel', defaultMessage: 'Símanúmer', description: 'Tenant Details phone input label', }, representativeLabel: { id: 'ra.application:tenantDetails.representativeLabel', defaultMessage: 'Þessi aðili er umboðsaðili leigjanda', description: 'Tenant Details representative label', }, nationalIdInvalidFormat: { id: 'ra.application:tenantDetails.nationalIdInvalidFormat', defaultMessage: 'Kennitala er ekki á réttu formi', description: 'Error message when national ID format is invalid', }, emailInvalidFormat: { id: 'ra.application:tenantDetails.emailInvalidFormat', defaultMessage: 'Netfang er ekki á réttu formi', description: 'Error message when email format is invalid', }, phoneInvalidFormat: { id: 'ra.application:tenantDetails.phoneInvalidFormat', defaultMessage: 'Símanúmer er ekki á réttu formi', description: 'Error message when phone number format is invalid', },libs/application/templates/rental-agreement/src/lib/messages/housing/landlordDetails.ts (1)
4-50: 🛠️ Refactor suggestion
Consider enhancing type safety and message organization.
The message definitions are well-structured with consistent IDs and comprehensive descriptions. However, consider these improvements:
- Add explicit type annotations for better maintainability
- Group related messages using nested objects
Here's how you could refactor the code:
import { defineMessages } from 'react-intl' + +type LandlordMessages = { + subSection: { + name: string + title: string + description: string + } + inputs: { + nationalId: string + name: string + email: string + phone: string + representative: string + } +} -export const landlordDetails = defineMessages({ +export const landlordDetails = defineMessages<LandlordMessages>({ + subSection: { 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', }, + }, + inputs: { nationalIdInputLabel: { // ... rest of the input-related messages }, // ... other input messages + } })Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingFireProtections.ts (3)
13-61: 🛠️ Refactor suggestion
Consider enhancing field descriptions with meaningful placeholders.
The current placeholder "0" could be more informative. Consider adding meaningful placeholders that indicate the expected input.
buildTextField({ id: 'rentalHousingFireProtectionsSmokeDetectors', title: m.housingFireProtections.smokeDetectorsLabel, - placeholder: '0', + placeholder: 'Enter number of smoke detectors', width: 'half', variant: 'number', }),Committable suggestion was skipped due to low confidence.
1-8: 🛠️ Refactor suggestion
Consider using named imports for messages.
Replace the wildcard import with specific named imports to improve tree-shaking and make dependencies more explicit.
-import * as m from '../../lib/messages' +import { housingFireProtections } from '../../lib/messages'Committable suggestion was skipped due to low confidence.
25-38:
⚠️ Potential issueAdd validation constraints for numeric inputs.
The smoke detectors and fire extinguisher fields should have minimum value validation to ensure valid inputs.
buildTextField({ id: 'rentalHousingFireProtectionsSmokeDetectors', title: m.housingFireProtections.smokeDetectorsLabel, placeholder: '0', width: 'half', variant: 'number', + props: { + min: 0, + required: true + } }),Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/lib/messages/housing/housingFireProtections.ts (3)
29-29:
⚠️ Potential issueFix typo in description
There's a typo in the description: "dectectors" should be "detectors"
- description: 'Smoke dectectors label', + description: 'Smoke detectors label',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.description: 'Smoke detectors label',
1-2: 🛠️ Refactor suggestion
Consider adding TypeScript types for messages
To improve type safety and align with the coding guidelines, consider using TypeScript types for the message definitions.
- 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'
37-41:
⚠️ Potential issueUpdate incorrect description for exitFireBlanketRequirements
The description mentions "Smoke detector and fire blanket requirements" but the message is about exits and fire blankets.
- 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.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', },libs/application/templates/rental-agreement/src/lib/messages/rentalPeriod/rentalPeriodDetails.ts (1)
25-29: 🛠️ Refactor suggestion
Consider creating a shared date placeholder message.
The placeholder text "Veldu dagsetningu" is duplicated for both start and end date fields. Consider extracting it into a shared message to maintain consistency and reduce duplication.
+ commonDatePlaceholder: { + id: 'ra.application:rentalPeriodDetails.commonDatePlaceholder', + defaultMessage: 'Veldu dagsetningu', + description: 'Common date field placeholder', + }, startDatePlaceholder: { - id: 'ra.application:rentalPeriodDetails.startDatePlaceholder', - defaultMessage: 'Veldu dagsetningu', - description: 'Start date placeholder', + id: 'ra.application:rentalPeriodDetails.startDatePlaceholder', + defaultMessage: '{commonDatePlaceholder}', + description: 'Start date placeholder', }, endDatePlaceholder: { - id: 'ra.application:rentalPeriodDetails.endDatePlaceholder', - defaultMessage: 'Veldu dagsetningu', - description: 'End date placeholder', + id: 'ra.application:rentalPeriodDetails.endDatePlaceholder', + defaultMessage: '{commonDatePlaceholder}', + description: 'End date placeholder', },Also applies to: 35-39
libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingTenantInfo.ts (1)
43-48: 🛠️ Refactor suggestion
Enhance email validation for better user experience.
Consider adding pattern validation or a custom validator to ensure proper email format beyond the basic HTML5 validation.
email: { component: 'input', label: m.tenantDetails.emailInputLabel, type: 'email', + pattern: '[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$', + validation: ['email'], width: 'half', },📝 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.email: { component: 'input', label: m.tenantDetails.emailInputLabel, type: 'email', pattern: '[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,}$', validation: ['email'], width: 'half', },libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingLandlordInfo.ts (2)
64-65:
⚠️ Potential issueAdd error handling for format functions.
The format functions should handle invalid input gracefully to prevent runtime errors.
format: { - phone: (value) => formatPhoneNumber(value), - nationalId: (value) => formatNationalId(value), + phone: (value) => { + try { + return value ? formatPhoneNumber(value) : '' + } catch (error) { + console.error('Error formatting phone number:', error) + return value + } + }, + nationalId: (value) => { + try { + return value ? formatNationalId(value) : '' + } catch (error) { + console.error('Error formatting national ID:', error) + return value + } + }, },📝 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.phone: (value) => { try { return value ? formatPhoneNumber(value) : '' } catch (error) { console.error('Error formatting phone number:', error) return value } }, nationalId: (value) => { try { return value ? formatNationalId(value) : '' } catch (error) { console.error('Error formatting national ID:', error) return value } },
25-61:
⚠️ Potential issueAdd field validation rules for required information.
Consider adding validation rules for critical fields:
- Name and National ID should be required
- Email should validate against a proper email pattern
- Phone number should validate against Icelandic phone number format
name: { component: 'input', label: m.landlordDetails.nameInputLabel, width: 'half', + required: true, + variant: 'text', }, nationalId: { component: 'input', label: m.landlordDetails.nationalIdInputLabel, format: '######-####', width: 'half', + required: true, + validate: (value) => { + if (!value) return m.landlordDetails.nationalIdRequired + if (!/^\d{6}-?\d{4}$/.test(value)) return m.landlordDetails.nationalIdInvalid + return undefined + }, }, email: { component: 'input', label: m.landlordDetails.emailInputLabel, type: 'email', width: 'half', + validate: (value) => { + if (value && !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) { + return m.landlordDetails.emailInvalid + } + return undefined + }, },Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingCondition.ts (3)
13-16: 🛠️ Refactor suggestion
Consider exporting TypeScript types for better reusability.
While the implementation is correct, consider exporting the form section type for better reusability across different NextJS apps.
Add type export above the implementation:
export type RentalHousingConditionType = ReturnType<typeof RentalHousingCondition>;
40-47: 🛠️ Refactor suggestion
Consider memoizing the condition function for better performance.
The condition function is recreated on every render. Consider memoizing it using useCallback for better performance.
import { useCallback } from 'react' // In your component: const inspectorCondition = useCallback((answers) => { const { inspectorOptions } = getApplicationAnswers(answers) return inspectorOptions === rentalHousingConditionInspector.INDEPENDENT_PARTY }, []) // Use in buildTextField: condition: inspectorCondition,
63-71:
⚠️ Potential issueReplace placeholder ID with a meaningful identifier.
The file upload field uses 'asdf' as its ID, which appears to be a placeholder. This should be replaced with a descriptive identifier that reflects its purpose.
- id: 'asdf', + id: 'rentalHousingConditionPhotos',📝 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.buildFileUploadField({ id: 'rentalHousingConditionPhotos', title: m.housingCondition.fileUploadTitle, uploadHeader: m.housingCondition.fileUploadTitle, uploadDescription: m.housingCondition.fileUploadDescription, uploadAccept: '.pdf, .doc, .docx, .rtf, .jpg, .jpeg, .png, .heic', uploadMultiple: true, forImageUpload: true, }),libs/application/templates/rental-agreement/src/lib/messages/housing/specialProvisions.ts (1)
1-3: 🛠️ Refactor suggestion
Add TypeScript type definitions for better type safety.
Consider adding explicit type definitions to improve type safety and documentation:
import { defineMessages } from 'react-intl' + type MessageDescriptor = { + id: string + defaultMessage: string + description: string + } + + type SpecialProvisionsMessages = { + subsection: Record<string, MessageDescriptor> + housingInfo: Record<string, MessageDescriptor> + housingRules: Record<string, MessageDescriptor> + } + - export const specialProvisions = { + export const specialProvisions: SpecialProvisionsMessages = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { defineMessages } from 'react-intl' type MessageDescriptor = { id: string defaultMessage: string description: string } type SpecialProvisionsMessages = { subsection: Record<string, MessageDescriptor> housingInfo: Record<string, MessageDescriptor> housingRules: Record<string, MessageDescriptor> } export const specialProvisions: SpecialProvisionsMessages = {libs/application/templates/rental-agreement/src/lib/messages/prerequisites.ts (1)
34-39: 🛠️ Refactor suggestion
Consider splitting bullet points into separate messages.
The current implementation combines multiple bullet points into a single message, which could make maintenance and translation more challenging. Consider splitting them into separate messages for better maintainability and translation accuracy.
- bullets: { - id: 'ra.application:prerequisites.intro.bullets#markdown', - defaultMessage: - '- Leigusamningur er skráður í Leiguskrá HMS þegar allir aðilar samningsins hafa undirritað rafrænt. \n- Skráning leigusamnings í Leiguskrá HMS er ein forsenda þess að leigjandi geti fengið greiddar húsnæðisbætur. \n- Hægt er að sækja um húsnæðisbætur samhliða skráningu á leigusamningnum og því þinglýsing óþörf. \n- Með rafrænni skráningu eru vottar óþarfi. \n- Staðfesting á brunavörnum og ástandi húsnæðis er hluti af leigusamningnum', - description: 'First bullet point about the application', - }, + bullet1: { + id: 'ra.application:prerequisites.intro.bullet1', + defaultMessage: 'Leigusamningur er skráður í Leiguskrá HMS þegar allir aðilar samningsins hafa undirritað rafrænt.', + description: 'Bullet point about digital signatures', + }, + bullet2: { + id: 'ra.application:prerequisites.intro.bullet2', + defaultMessage: 'Skráning leigusamnings í Leiguskrá HMS er ein forsenda þess að leigjandi geti fengið greiddar húsnæðisbætur.', + description: 'Bullet point about housing benefits requirement', + }, + // ... continue for remaining bulletsCommittable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodAmount.ts (3)
72-79: 🛠️ Refactor suggestion
Use Date utility for consistent date handling.
Instead of manual string manipulation, consider using a date utility for better maintainability.
buildDateField({ id: 'rentalAmountIndexDate', title: m.rentalAmount.indexDateLabel, maxDate: new Date(), - defaultValue: new Date().toISOString().substring(0, 10), + defaultValue: format(new Date(), 'yyyy-MM-dd'), width: 'half', condition: rentalAmountIndexIsConnected, }),Consider importing
formatfrom a date utility library likedate-fns.Committable suggestion was skipped due to low confidence.
24-27: 🛠️ Refactor suggestion
Add TypeScript type safety to the utility function.
The function could benefit from explicit TypeScript types for better maintainability and type safety.
-function rentalAmountIndexIsConnected(answers: FormValue) { +function rentalAmountIndexIsConnected(answers: FormValue): boolean { const { isRentalAmountIndexConnected } = getApplicationAnswers(answers) - return isRentalAmountIndexConnected !== undefined + return isRentalAmountIndexConnected === true }📝 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 rentalAmountIndexIsConnected(answers: FormValue): boolean { const { isRentalAmountIndexConnected } = getApplicationAnswers(answers) return isRentalAmountIndexConnected === true }
44-51: 🛠️ Refactor suggestion
Add minimum value validation for the rental amount.
While the maxLength is set, there's no validation to ensure the rental amount is positive.
buildTextField({ id: 'rentalAmount.amount', title: m.rentalAmount.inputLabel, placeholder: m.rentalAmount.inputPlaceholder, variant: 'currency', maxLength: 14, + min: 0, required: true, }),📝 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: 'rentalAmount.amount', title: m.rentalAmount.inputLabel, placeholder: m.rentalAmount.inputPlaceholder, variant: 'currency', maxLength: 14, min: 0, required: true, }),libs/application/templates/rental-agreement/src/lib/utils.ts (2)
20-62: 🛠️ Refactor suggestion
Enhance type safety and error handling in getApplicationAnswers.
The function could benefit from:
- Type assertions or runtime checks for retrieved values
- Error handling for missing answers
- More modular structure for better reusability
Consider this improved implementation:
interface ApplicationAnswers { propertyCategoryTypeOptions: rentalHousingCategoryTypes | null; propertyCategoryClassOptions: rentalHousingCategoryClass | null; inspectorOptions: rentalHousingConditionInspector | null; isRentalAmountIndexConnected: AnswerOptions | null; rentalAmountIndexTypesOptions: rentalAmountIndexTypes | null; rentalAmountPaymentDateOptions: rentalAmountPaymentDateOptions | null; } const getAnswerValue = <T>(answers: Application['answers'], path: string): T | null => { try { return getValueViaPath<T>(answers, path); } catch (error) { console.warn(`Failed to retrieve answer for path: ${path}`, error); return null; } }; export const getApplicationAnswers = (answers: Application['answers']): ApplicationAnswers => ({ propertyCategoryTypeOptions: getAnswerValue(answers, 'registerPropertyCategoryType'), propertyCategoryClassOptions: getAnswerValue(answers, 'registerPropertyCategoryClass'), inspectorOptions: getAnswerValue(answers, 'rentalHousingConditionInspector'), isRentalAmountIndexConnected: getAnswerValue(answers, 'isRentalAmountIndexConnected'), rentalAmountIndexTypesOptions: getAnswerValue(answers, 'rentalAmountIndexTypes'), rentalAmountPaymentDateOptions: getAnswerValue(answers, 'rentalAmountPaymentDate'), })
14-19:
⚠️ Potential issueAdd input validation and handle edge cases in string utilities.
The string manipulation functions need more robust error handling:
insertAtshould validate input parameters:
- Check if str is defined
- Ensure pos is within bounds
formatNationalIdhas potential issues:
- Doesn't handle empty strings properly
- May fail with inputs shorter than 6 characters
- Multiple hyphens aren't handled consistently
- Returning '-' for invalid input could mask errors
Consider this safer implementation:
-export const insertAt = (str: string, sub: string, pos: number) => - `${str.slice(0, pos)}${sub}${str.slice(pos)}` +export const insertAt = (str: string, sub: string, pos: number): string => { + if (!str || pos < 0 || pos > str.length) { + throw new Error('Invalid input parameters'); + } + return `${str.slice(0, pos)}${sub}${str.slice(pos)}`; +} -export const formatNationalId = (nationalId: string) => - insertAt(nationalId.replace('-', ''), '-', 6) || '-' +export const formatNationalId = (nationalId: string): string => { + if (!nationalId || nationalId.length < 7) { + throw new Error('Invalid national ID format'); + } + const cleaned = nationalId.replace(/-/g, ''); + return insertAt(cleaned, '-', 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 insertAt = (str: string, sub: string, pos: number): string => { if (!str || pos < 0 || pos > str.length) { throw new Error('Invalid input parameters'); } return `${str.slice(0, pos)}${sub}${str.slice(pos)}`; } export const formatNationalId = (nationalId: string): string => { if (!nationalId || nationalId.length < 7) { throw new Error('Invalid national ID format'); } const cleaned = nationalId.replace(/-/g, ''); return insertAt(cleaned, '-', 6); }libs/application/templates/rental-agreement/src/forms/rentalHousingInfo/rentalHousingPropertyInfo.ts (4)
66-71: 🛠️ Refactor suggestion
Consider memoizing postal code options.
The postal code options mapping could be optimized to prevent unnecessary re-renders.
- options: () => { - return postalCodes.map((code) => { - return { value: `${code}`, label: `${code}` } - }) - }, + options: () => postalCodes.map((code) => ({ + value: code.toString(), + label: code.toString() + })),📝 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.options: () => postalCodes.map((code) => ({ value: code.toString(), label: code.toString() })), }),
167-174: 🛠️ Refactor suggestion
Extract complex condition logic to a utility function.
The condition logic for category class group could be more readable if extracted to a separate function.
+ const isSpecialGroupsCategory = (answers: object) => { + const { propertyCategoryClassOptions } = getApplicationAnswers(answers) + return propertyCategoryClassOptions === rentalHousingCategoryClass.SPECIAL_GROUPS + } - condition: (answers) => { - const { propertyCategoryClassOptions } = getApplicationAnswers(answers) - return ( - propertyCategoryClassOptions === rentalHousingCategoryClass.SPECIAL_GROUPS - ) - }, + condition: isSpecialGroupsCategory,📝 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 isSpecialGroupsCategory = (answers: object) => { const { propertyCategoryClassOptions } = getApplicationAnswers(answers) return propertyCategoryClassOptions === rentalHousingCategoryClass.SPECIAL_GROUPS } condition: isSpecialGroupsCategory,
79-85:
⚠️ Potential issueAdd input validation for property size.
The size field should include numeric validation and unit constraints.
buildTextField({ id: 'registerPropertyInfoSize', title: messagesInfo.sizeLabel, placeholder: '', width: 'half', required: true, + variant: 'number', + format: '###.##', + minValue: 0, + maxValue: 1000, }),📝 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: 'registerPropertyInfoSize', title: messagesInfo.sizeLabel, placeholder: '', width: 'half', required: true, variant: 'number', format: '###.##', minValue: 0, maxValue: 1000, }),
99-129: 🛠️ Refactor suggestion
Enhance address validation and string formatting.
The current implementation has a few areas for improvement:
- The empty address check could be more robust
- String concatenation could be simplified using template literals
- condition: (answers) => answers.registerPropertyInfoAddress !== '', + condition: (answers) => Boolean(answers.registerPropertyInfoAddress?.trim()), - answers.registerPropertyInfoAddress - ? `${answers.registerPropertyInfoAddress.toString()}, ${answers.registerPropertyInfoMunicipality.toString()}` - : '', + answers.registerPropertyInfoAddress + ? `${answers.registerPropertyInfoAddress}, ${answers.registerPropertyInfoMunicipality}` + : '',📝 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) => Boolean(answers.registerPropertyInfoAddress?.trim()), children: [ buildStaticTableField({ title: messagesInfoSummary.pageTitle, description: messagesInfoSummary.pageDescription, header: [ messagesInfoSummary.tableHeaderPropertyId, messagesInfoSummary.tableHeaderAddress, messagesInfoSummary.tableHeaderUnitId, messagesInfoSummary.tableHeaderSize, messagesInfoSummary.tableHeaderNumOfRooms, ], rows({ answers }) { return [ [ answers.registerPropertyInfoPropertyId ? `F${answers.registerPropertyInfoPropertyId}` : '', answers.registerPropertyInfoAddress ? `${answers.registerPropertyInfoAddress}, ${answers.registerPropertyInfoMunicipality}` : '', answers.registerPropertyInfoUnitId ? `${answers.registerPropertyInfoUnitId.toString()}` : '', answers.registerPropertyInfoSize ? `${answers.registerPropertyInfoSize.toString()} m²` : '', answers.registerPropertyInfoNumOfRooms ? `${answers.registerPropertyInfoNumOfRooms.toString()}` : '', ],libs/application/templates/rental-agreement/src/lib/messages/housing/registerProperty.ts (2)
19-23: 🛠️ Refactor suggestion
Parameterize external links in translations.
Consider extracting the HMS website URL to a configuration value and using message parameters instead of hardcoding it in the translation.
pageDescription: { id: 'ra.application:registerProperty.info.pageDescription', defaultMessage: - 'Finndu eignina með fasteignanúmeri eða heimilisfangi. Nánari upplýsingar er að finna í [fasteignaskrá HMS](https://hms.is/fasteignir).', + 'Finndu eignina með fasteignanúmeri eða heimilisfangi. Nánari upplýsingar er að finna í [fasteignaskrá HMS]({hmsUrl}).', description: 'Register property page description', },Committable suggestion was skipped due to low confidence.
1-3: 🛠️ Refactor suggestion
Add TypeScript type definitions for better type safety and maintainability.
Consider adding explicit type definitions to improve type safety and documentation.
import { defineMessages } from 'react-intl' + import { MessageDescriptor } from 'react-intl' + interface PropertyMessages { + subsection: Record<string, MessageDescriptor> + info: Record<string, MessageDescriptor> + infoSummary: Record<string, MessageDescriptor> + category: Record<string, MessageDescriptor> + } - export const registerProperty = { + export const registerProperty: PropertyMessages = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { defineMessages } from 'react-intl' import { MessageDescriptor } from 'react-intl' interface PropertyMessages { subsection: Record<string, MessageDescriptor> info: Record<string, MessageDescriptor> infoSummary: Record<string, MessageDescriptor> category: Record<string, MessageDescriptor> } export const registerProperty: PropertyMessages = {libs/application/templates/rental-agreement/src/lib/messages/rentalInfo/securityDeposit.ts (5)
26-29:
⚠️ Potential issueUpdate placeholder tooltip text
The tooltip message contains placeholder text "Tool tip" which should be replaced with actual helpful content.
typeHeaderToolTip: { id: 'ra.application:securityDeposit.typeHeaderToolTip', - defaultMessage: 'Tool tip', + defaultMessage: 'Veldu tegund tryggingar fyrir leigusamninginn', description: 'security deposit type header tool tip', },📝 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: 'ra.application:securityDeposit.typeHeaderToolTip', defaultMessage: 'Veldu tegund tryggingar fyrir leigusamninginn', description: 'security deposit type header tool tip', },
127-131:
⚠️ Potential issueUpdate placeholder tooltip text
The tooltip message contains placeholder text "Tool tip" which should be replaced with actual helpful content.
amountHeaderToolTip: { id: 'ra.application:securityDeposit.amountHeaderToolTip', - defaultMessage: 'Tool tip', + defaultMessage: 'Veldu upphæð tryggingar fyrir leigusamninginn', description: 'security deposit amount header tool tip', },📝 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.amountHeaderToolTip: { id: 'ra.application:securityDeposit.amountHeaderToolTip', defaultMessage: 'Veldu upphæð tryggingar fyrir leigusamninginn', description: 'security deposit amount header tool tip', },
142-146:
⚠️ Potential issueFix duplicate message ID for 2-month selection
The message ID for 2-month selection incorrectly uses the same ID as 1-month selection.
amountSelection2Month: { - id: 'ra.application:securityDeposit.amountSelection1Month', + id: 'ra.application:securityDeposit.amountSelection2Month', defaultMessage: '2 mánuðir af leiguverði', description: 'security deposit amount selection 2 months', },📝 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.amountSelection2Month: { id: 'ra.application:securityDeposit.amountSelection2Month', defaultMessage: '2 mánuðir af leiguverði', description: 'security deposit amount selection 2 months', },
86-90:
⚠️ Potential issueFix incorrect placeholder text for third party guarantee
The placeholder text incorrectly asks for an institution name instead of the guarantor's name.
thirdPartyGuaranteeInfoPlaceholder: { id: 'ra.application:securityDeposit.thirdPartyGuaranteeInfoPlaceholder', - defaultMessage: 'Skrifaðu hér hjá hvaða stofnun ábyrgðin er', + defaultMessage: 'Skrifaðu hér nafn ábyrgðaraðila', description: 'security deposit third party guarantee info 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.thirdPartyGuaranteeInfoPlaceholder: { id: 'ra.application:securityDeposit.thirdPartyGuaranteeInfoPlaceholder', defaultMessage: 'Skrifaðu hér nafn ábyrgðaraðila', description: 'security deposit third party guarantee info placeholder', },
224-228:
⚠️ Potential issueFix spelling: "capitol" should be "capital"
The message ID and description contain a spelling error.
- amountOtherCapitolError: { - id: 'ra.application:securityDeposit.amountOtherCapitolError', + amountOtherCapitalError: { + id: 'ra.application:securityDeposit.amountOtherCapitalError', defaultMessage: 'Upphæð tryggingar má ekki vera meira en 3x leiguverð', - description: 'security deposit amount other capitol error', + description: 'security deposit amount other capital error', },📝 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.amountOtherCapitalError: { id: 'ra.application:securityDeposit.amountOtherCapitalError', defaultMessage: 'Upphæð tryggingar má ekki vera meira en 3x leiguverð', description: 'security deposit amount other capital error', },libs/application/templates/rental-agreement/src/lib/RentalAgreementTemplate.ts (2)
38-38: 🛠️ Refactor suggestion
Review lifecycle pruning duration
Both the
PREREQUISITESandDRAFTstates have thelifecycleset topruneAfterDays(1), which means the application will be pruned after one day if not completed. This duration might be too short for applicants to finish their application, potentially leading to a poor user experience.Consider increasing the pruning duration to give applicants more time to complete the application. For example:
38- lifecycle: pruneAfterDays(1), 38+ lifecycle: pruneAfterDays(7), 68- lifecycle: pruneAfterDays(1), 68+ lifecycle: pruneAfterDays(7),Also applies to: 68-68
84-86:
⚠️ Potential issueUse consistent state references in transitions
In the
'on'event of theDRAFTstate, the target state is specified as the string'draft'. For consistency and to avoid potential typos, consider using theStates.DRAFTconstant instead.Apply this diff to fix the inconsistency:
84 [DefaultEvents.SUBMIT]: { 85- target: 'draft', 85+ target: States.DRAFT, 86 },📝 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, },libs/application/templates/rental-agreement/src/lib/dataSchema.ts (6)
158-158: 🛠️ Refactor suggestion
Add validation for
approveExternalDataThe
approveExternalDatafield lacks a validation error message. Adding a custom message will improve user feedback when the field is not approved.Apply this diff to include an error message:
-approveExternalData: z.boolean().refine((v) => v), +approveExternalData: z.boolean().refine((v) => v, { + message: 'You must approve the external data to proceed.', +}),📝 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.approveExternalData: z.boolean().refine((v) => v, { message: 'You must approve the external data to proceed.', }),
11-17: 🛠️ Refactor suggestion
Simplify the
checkIfNegativefunctionThe
checkIfNegativefunction can be simplified by returning the boolean expression directly, eliminating the unnecessaryif...elsestatement.Apply this diff to simplify the function:
-const checkIfNegative = (inputNumber: string) => { - if (Number(inputNumber) < 0) { - return false - } else { - return true - } -} +const checkIfNegative = (inputNumber: string) => Number(inputNumber) >= 0📝 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 checkIfNegative = (inputNumber: string) => Number(inputNumber) >= 0
160-164: 🛠️ Refactor suggestion
Improve error handling for
nationalIdvalidationWhen validating
nationalId, the error message is set inparams, but it would be clearer to set it directly in themessagefield.Apply this diff to set the error message:
.refine((val) => (val ? kennitala.isValid(val) : false), { - params: m.dataSchema.nationalId, + message: m.dataSchema.nationalId.defaultMessage, })📝 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.nationalId: z .string() .refine((val) => (val ? kennitala.isValid(val) : false), { message: m.dataSchema.nationalId.defaultMessage, }),
34-38: 🛠️ Refactor suggestion
Consider using
z.number()for numeric validationCurrently, the
amountfield is defined as a string and uses custom refinements to check for emptiness and negativity. Usingz.number()provides built-in number validation and simplifies the schema.Apply this diff to update the schema:
-amount: z - .string() - .refine((x) => Boolean(x), { - params: requiredErrorMsg, - }) - .refine((x) => checkIfNegative(x), { params: negativeNumberError }), +amount: z.number().nonnegative({ + message: negativeNumberError.defaultMessage, +})Committable suggestion was skipped due to low confidence.
166-166:
⚠️ Potential issueRename the
asdfproperty to a meaningful nameThe property
asdfappears to be a placeholder and should be renamed to reflect its purpose in the schema.For example, if this field is intended to store uploaded files, it could be renamed to
attachments:- asdf: z.array(FileSchema), + attachments: z.array(FileSchema),Committable suggestion was skipped due to low confidence.
59-154:
⚠️ Potential issueReplace placeholder error messages with meaningful content
In the
superRefinemethod, themessagefield is set to'Custom error message'in multiple places. This placeholder should be replaced with actual error messages to provide clarity during validation.Apply this diff to update the error messages:
ctx.addIssue({ code: z.ZodIssueCode.custom, - message: 'Custom error message', + message: params.defaultMessage, params: /* respective error message object */, path: [/* respective path */], })Ensure that each
paramsobject is correctly referenced, andmessageis set toparams.defaultMessagefrom the corresponding error message inm.securityDeposit.Committable suggestion was skipped due to low confidence.
libs/application/templates/rental-agreement/src/forms/rentalPeriod/rentalPeriodSecurityDeposit.ts (2)
88-94: 🛠️ Refactor suggestion
Refactor repeated condition logic into a helper function
The
conditionfunctions used in multiple fields are repeating similar logic to checksecurityDeposit.securityType. Extracting this logic into a utility function can enhance code reusability and readability across the form.You can define a helper function at the top of your file:
const isSecurityType = (answers: FormValue, type: string) => { const securityDeposit = answers.securityDeposit as FormValue return securityDeposit?.securityType === type }Then, update your
conditionproperties:-condition: (answers) => { - const securityDeposit = answers.securityDeposit as FormValue - return ( - securityDeposit && - Boolean(securityDeposit.securityType) && - securityDeposit.securityType === 'bankGuarantee' - ) -}, +condition: (answers) => isSecurityType(answers, 'bankGuarantee'),This refactoring simplifies the conditions and makes the codebase easier to maintain.
Also applies to: 102-110, 117-124, 131-138, 145-152, 159-166, 207-217
14-14:
⚠️ Potential issueRemove unused constant 'messages'
The constant
messagesdeclared on line 14 is not used anywhere in the file. Removing it will improve code cleanliness and aid in effective tree-shaking during bundling.Apply this diff to remove the unused constant:
-const messages = m.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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## rental-agreement-application #16661 +/- ##
================================================================
- Coverage 36.71% 36.65% -0.06%
================================================================
Files 6850 6859 +9
Lines 142065 143015 +950
Branches 40558 40892 +334
================================================================
+ Hits 52153 52421 +268
- Misses 89912 90594 +682 Flags with carried forward coverage won't be shown. Click here to find out more. see 191 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 67 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
https://app.asana.com/0/1208271027457465/1208501204323908
Checklist:
Summary by CodeRabbit
New Features
Documentation
Configuration