fix(income-plan): amend dynamic eligible messages #16918
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the logic in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
ylfahfa
added
the
deprecated:automerge
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
libs/application/templates/social-insurance-administration/income-plan/src/lib/constants.ts (1)
13-13: LGTM! Consider enhancing type safety.The new constant follows the established naming pattern and is well-placed among other status-related constants. The individual export enables effective tree-shaking.
Consider using a string literal union type for better type safety and autocompletion:
export type ApplicationStatus = 'INCOME_PLANS_CLOSED' | 'NO_ACTIVE_APPLICATIONS'; export const INCOME_PLANS_CLOSED: ApplicationStatus = 'INCOME_PLANS_CLOSED'; export const NO_ACTIVE_APPLICATIONS: ApplicationStatus = 'NO_ACTIVE_APPLICATIONS';libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.ts (1)
138-139: Consider refactoring for better maintainabilityWhile the logic is correct, the nested ternary operators make the code harder to read and maintain. Consider refactoring this function for better clarity and maintainability.
Here's a suggested improvement:
-export const eligibleText = (externalData: ExternalData) => { +export const getEligibilityMessage = (externalData: ExternalData): string => { const { isEligible } = getApplicationExternalData(externalData) - return isEligible.reasonCode === INCOME_PLANS_CLOSED - ? incomePlanFormMessage.pre.isNotEligibleClosedDescription - : isEligible.reasonCode === NO_ACTIVE_APPLICATIONS - ? incomePlanFormMessage.pre.isNotEligibleNoActiveApplicationDescription - : incomePlanFormMessage.pre.isNotEligibleDescription + + switch (isEligible.reasonCode) { + case INCOME_PLANS_CLOSED: + return incomePlanFormMessage.pre.isNotEligibleClosedDescription + case NO_ACTIVE_APPLICATIONS: + return incomePlanFormMessage.pre.isNotEligibleNoActiveApplicationDescription + default: + return incomePlanFormMessage.pre.isNotEligibleDescription + } }Changes suggested:
- Renamed function to better reflect its purpose (handling error messages)
- Added return type annotation
- Replaced nested ternaries with more readable switch statement
libs/application/templates/social-insurance-administration/income-plan/src/forms/IncomePlanForm.ts (2)
Line range hint
72-84: Fix potential null reference when accessing latestIncomePlan.yearThere's a risk of a runtime error when accessing
latestIncomePlan.yeariflatestIncomePlanis empty. The empty check is performed but not used when accessing the year property.Apply this diff to fix the issue:
return { ...baseMessage, values: { - incomePlanYear: latestIncomePlan.year, + incomePlanYear: hasLatestIncomePlan ? latestIncomePlan.year : undefined, }, }
Line range hint
249-266: Extract repeated condition logic into a helper functionThe condition for checking uneven employment income is duplicated across multiple fields. This should be extracted into a helper function for better maintainability.
Create a new helper function:
const isUnevenEmploymentIncome = (activeField: any) => activeField?.unevenIncomePerYear?.[0] !== YES || (activeField?.incomeCategory !== INCOME && activeField?.unevenIncomePerYear?.[0] === YES)Then use it in the conditions:
condition: (_, activeField) => { - const unevenAndEmploymentIncome = - activeField?.unevenIncomePerYear?.[0] !== YES || - (activeField?.incomeCategory !== INCOME && - activeField?.unevenIncomePerYear?.[0] === YES) - return ( activeField?.income === RatioType.MONTHLY && activeField?.currency !== ISK && - unevenAndEmploymentIncome + isUnevenEmploymentIncome(activeField) ) }Also applies to: 288-305
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
libs/application/templates/social-insurance-administration/income-plan/src/forms/IncomePlanForm.ts(2 hunks)libs/application/templates/social-insurance-administration/income-plan/src/lib/constants.ts(1 hunks)libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.ts(2 hunks)libs/application/templates/social-insurance-administration/income-plan/src/lib/messages.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/application/templates/social-insurance-administration/income-plan/src/forms/IncomePlanForm.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/income-plan/src/lib/constants.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/income-plan/src/lib/messages.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (4)
libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.ts (2)
11-11: LGTM! Clean import addition
The new constant import follows the existing pattern and is logically grouped with related constants.
11-11: Verify the impact of these changes
Let's ensure all usages of eligibleText are aware of the new message case.
Also applies to: 138-139
✅ Verification successful
No impact on consumers - constants are properly handled
The eligibleText function already handles both INCOME_PLANS_CLOSED and NO_ACTIVE_APPLICATIONS cases correctly, returning appropriate messages for each case. The function implementation shows proper handling of all scenarios:
- When
reasonCodeisINCOME_PLANS_CLOSED - When
reasonCodeisNO_ACTIVE_APPLICATIONS - Default case for other scenarios
The only consumer found (Prerequisites.ts) uses the function as intended, simply displaying the returned message.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of eligibleText to ensure consumers handle all cases
rg "eligibleText" --type ts
# Search for tests to ensure new case is covered
rg -t test "eligibleText|NO_ACTIVE_APPLICATIONS"
Length of output: 585
Script:
#!/bin/bash
# Search for the function implementation and its test coverage
rg "eligibleText" -A 10 --type typescript
# Search for the constants usage
rg "NO_ACTIVE_APPLICATIONS|INCOME_PLANS_CLOSED" --type typescript
# Look for test files with .spec or .test extension
fd ".(spec|test)\.(ts|tsx)$" | xargs rg "eligibleText|NO_ACTIVE_APPLICATIONS"
Length of output: 259
Script:
#!/bin/bash
# Search for the function implementation and its test coverage
rg "eligibleText" -A 10
# Search for the constants usage
rg "NO_ACTIVE_APPLICATIONS|INCOME_PLANS_CLOSED"
# Look for test files
fd ".(spec|test)\.(ts|tsx)$"
Length of output: 76494
Script:
#!/bin/bash
# Search for the implementation of eligibleText function and its test coverage
rg -A 10 "export const eligibleText" libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.ts
# Search for the test file content
rg -A 10 "eligibleText" libs/application/templates/social-insurance-administration/income-plan/src/lib/incomePlanUtils.spec.ts
# Search for all usages of the constants
rg "NO_ACTIVE_APPLICATIONS|INCOME_PLANS_CLOSED" -A 2
Length of output: 2489
libs/application/templates/social-insurance-administration/income-plan/src/lib/messages.ts (1)
27-29: LGTM! Clear and consistent error message.
The modified error message maintains a clear structure and consistent formatting while improving clarity about the reason for ineligibility.
libs/application/templates/social-insurance-administration/income-plan/src/forms/IncomePlanForm.ts (1)
Line range hint 1-600: Implementation follows best practices and guidelines
The form implementation:
- Uses TypeScript effectively for type safety
- Is modular and reusable across different NextJS apps
- Handles complex conditional logic in a structured way
- Provides clear field labels and tooltips for better user experience
libs/application/templates/social-insurance-administration/income-plan/src/lib/messages.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16918 +/- ##
==========================================
- Coverage 36.46% 36.34% -0.13%
==========================================
Files 6903 6904 +1
Lines 144575 146171 +1596
Branches 41282 42111 +829
==========================================
+ Hits 52715 53120 +405
- Misses 91860 93051 +1191
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 95 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 32 Total Test Services: 0 Failed, 31 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
* use correct variable to determine ip year * add option to dynamic eligible text * format * fix text --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Updates
These changes aim to streamline user interactions and improve the overall experience with the income plan functionalities.