-
Notifications
You must be signed in to change notification settings - Fork 61
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(web): Pension Calculator - "After September 1 2025" year option #17117
base: main
Are you sure you want to change the base?
feat(web): Pension Calculator - "After September 1 2025" year option #17117
Conversation
WalkthroughThe pull request introduces substantial modifications to the pension calculation components, particularly in Changes
Possibly related PRs
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
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17117 +/- ##
==========================================
- Coverage 35.76% 35.73% -0.04%
==========================================
Files 6931 6934 +3
Lines 147959 148114 +155
Branches 42168 42264 +96
==========================================
Hits 52924 52924
- Misses 95035 95190 +155
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Affected services are: api,web, Deployed services: api,web,consultation-portal,services-bff-portals-admin,services-bff-portals-my-pages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code owned files LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
libs/api/domains/social-insurance/src/lib/dtos/pensionCalculation.input.ts (1)
10-12
: Consider removing theNewSystem
enum value if it's redundantWith the addition of
NewSystemDisability
,NewSystemPartialDisability
, andNewSystemMedicalAndRehabilitation
, theNewSystem
enum value on line 6 might no longer be necessary. Keeping unused or redundant enum values can cause confusion.If
BasePensionType.NewSystem
is no longer used, consider removing it to maintain clarity and prevent potential confusion in the codebase.apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
975-1006
: Consider using consistent casing in translation keys.While the translation strings are well-defined, the translation keys use inconsistent casing patterns. For better maintainability, consider standardizing the casing across all keys.
Example of consistent casing:
- 'REIKNH.SJUKRAOGENDURH_HUPPBOT_2025' + 'REIKNH.SJUKRA_OG_ENDURH_HUPP_BOT_2025' - 'REIKNH.HLUTAORORKA' + 'REIKNH.HLUTA_ORORKA'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
(10 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx
(3 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts
(3 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/utils.ts
(1 hunks)libs/api/domains/social-insurance/src/lib/dtos/pensionCalculation.input.ts
(1 hunks)libs/api/domains/social-insurance/src/lib/utils.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/api/domains/social-insurance/src/lib/dtos/pensionCalculation.input.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/api/domains/social-insurance/src/lib/utils.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."
apps/web/screens/Organization/SocialInsuranceAdministration/utils.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (3)
apps/web/screens/Organization/SocialInsuranceAdministration/utils.ts (1)
157-161
: Implementation of is2025FormPreviewActive
and NEW_SYSTEM_TAKES_PLACE_DATE
is correct
The utility function is2025FormPreviewActive
and the constant NEW_SYSTEM_TAKES_PLACE_DATE
are properly implemented.
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (2)
20-25
: LGTM! Translation string for 2025 preview option is well-defined.
The translation key, default message, and description are clear and follow the established pattern.
63-80
: LGTM! New pension type labels are comprehensive and well-structured.
The translation strings for the new pension system types (Disability Pension, Partial Disability Pension, and Medical/Rehabilitation) are properly defined with clear descriptions and appropriate default messages.
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
Show resolved
Hide resolved
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
Show resolved
Hide resolved
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
Outdated
Show resolved
Hide resolved
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculatorResults.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (2)
182-193
: Add error handling for empty options array.While the implementation is correct, consider adding error handling to ensure the options array is never empty, as this could affect the UI rendering.
const allCalculatorsOptions = useMemo(() => { const options = [...dateOfCalculationsOptions] + if (!options.length) { + console.warn('No calculator options available') + return [{ label: 'Default', value: new Date().toISOString() }] + } if (is2025FormPreviewActive(customPageData)) { options.unshift({ label: formatMessage(translationStrings.form2025PreviewLabel), value: NEW_SYSTEM_TAKES_PLACE_DATE.toISOString(), }) } return options }, [customPageData, dateOfCalculationsOptions, formatMessage])
606-630
: Consider extracting system transition logic.The date selection handler contains complex nested conditions. Consider extracting this logic into a separate function for better maintainability.
+const handleSystemTransition = ( + newDate: string, + currentDate: string, + isNewSystemActive: boolean, + setValue: (field: keyof CalculationInput, value: BasePensionType) => void +) => { + if (newDate === NEW_SYSTEM_TAKES_PLACE_DATE.toISOString()) { + setValue('typeOfBasePension', BasePensionType.NewSystemDisability) + } else if (currentDate === NEW_SYSTEM_TAKES_PLACE_DATE.toISOString()) { + setValue('typeOfBasePension', BasePensionType.Retirement) + } +} options={allCalculatorsOptions} onSelect={(option) => { if (option) { setDateOfCalculations(option.value) - if (isNewSystemActive) { - if (option.value === NEW_SYSTEM_TAKES_PLACE_DATE.toISOString()) { - methods.setValue('typeOfBasePension', BasePensionType.NewSystemDisability) - } else if (dateOfCalculations === NEW_SYSTEM_TAKES_PLACE_DATE.toISOString()) { - methods.setValue('typeOfBasePension', BasePensionType.Retirement) - } - } + handleSystemTransition( + option.value, + dateOfCalculations, + isNewSystemActive, + methods.setValue + ) } }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
(11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (7)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (7)
60-61
: LGTM: New utility imports for 2025 system support.
The imports are correctly added and align with the feature requirements.
75-78
: LGTM: Comprehensive disability assessment checks.
The function has been properly updated to include all disability-related pension types for both current and new system.
139-147
: Ensure dateOfCalculationsOptions is not empty before accessing its first element.
The initialization of dateOfCalculations accesses dateOfCalculationsOptions[0].value without verifying that the array contains elements.
195-198
: LGTM: Clean implementation of new system activation check.
The state correctly determines if the 2025 system is active based on both feature flag and date selection.
404-426
: Potential infinite loop in useEffect.
The effect updates typeOfBasePension while also depending on it, which could cause an infinite loop.
484-487
: Ensure safe access to dateOfCalculationsOptions.
921-924
: LGTM: Consistent disability type checks.
The disability assessment conditions have been properly updated to include new system types.
Also applies to: 937-941
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
20-25
: Consider parameterizing the date in the translation string.The date "1. september 2025" is hardcoded in the translation string. Consider using a placeholder to make it more maintainable if the date needs to change in the future.
Example:
- defaultMessage: 'Eftir 1. september 2025', + defaultMessage: 'Eftir {date}',apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (2)
75-78
: Consider refactoring for better maintainability.The function could be simplified by grouping related pension types into a constant array.
+const DISABILITY_PENSION_TYPES = [ + BasePensionType.Disability, + BasePensionType.Rehabilitation, + BasePensionType.NewSystemDisability, + BasePensionType.NewSystemPartialDisability, + BasePensionType.NewSystemMedicalAndRehabilitation, +] as const; const hasDisabilityAssessment = ( typeOfBasePension: BasePensionType | null | undefined, ) => { - return ( - typeOfBasePension === BasePensionType.Disability || - typeOfBasePension === BasePensionType.Rehabilitation || - typeOfBasePension === BasePensionType.NewSystemDisability || - typeOfBasePension === BasePensionType.NewSystemPartialDisability || - typeOfBasePension === BasePensionType.NewSystemMedicalAndRehabilitation - ) + return DISABILITY_PENSION_TYPES.includes(typeOfBasePension as BasePensionType) }
Line range hint
925-945
: Optimize repeated pension type checks.The repeated pension type checks could be simplified using the suggested DISABILITY_PENSION_TYPES constant.
-typeOfBasePension === BasePensionType.Disability || -typeOfBasePension === BasePensionType.NewSystemDisability || -typeOfBasePension === BasePensionType.NewSystemPartialDisability +DISABILITY_PENSION_TYPES.includes(typeOfBasePension as BasePensionType)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
(11 hunks)apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (6)
apps/web/screens/Organization/SocialInsuranceAdministration/translationStrings.ts (3)
63-80
: LGTM! Clear and consistent pension type labels.
The new pension type labels are well-structured and properly documented with clear descriptions indicating their purpose in the new system.
456-460
: LGTM! Clear form title for the new calculator.
The title is specific and accurately describes the purpose of the 2025 calculator view.
980-1011
: LGTM! Comprehensive coverage of benefit combinations.
The translation strings thoroughly cover all possible combinations of benefits in the new system, maintaining consistency with the existing pattern. The descriptions are clear and helpful for future maintenance.
apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (3)
1-1
: LGTM: Import and constant declarations are well-structured.
The new imports and constants are properly organized and follow TypeScript best practices.
Also applies to: 60-61
139-147
:
Ensure safe initialization of dateOfCalculations.
The current implementation might cause issues if dateOfCalculationsOptions is empty.
const [dateOfCalculations, setDateOfCalculations] = useQueryState(
'dateOfCalculations',
{
defaultValue:
- methods.formState.defaultValues?.dateOfCalculations ??
- dateOfCalculationsOptions[0].value,
+ methods.formState.defaultValues?.dateOfCalculations ??
+ (dateOfCalculationsOptions[0]?.value || new Date().toISOString()),
},
)
404-426
:
Prevent potential infinite loop in useEffect.
The current implementation might cause an infinite loop due to the dependency on typeOfBasePension.
- useEffect(() => {
+ useEffect(() => {
+ const currentType = methods.getValues('typeOfBasePension');
if (isNewSystemActive) {
if (
- typeOfBasePension !== BasePensionType.NewSystemDisability &&
- typeOfBasePension !== BasePensionType.NewSystemPartialDisability &&
- typeOfBasePension !== BasePensionType.NewSystemMedicalAndRehabilitation
+ currentType !== BasePensionType.NewSystemDisability &&
+ currentType !== BasePensionType.NewSystemPartialDisability &&
+ currentType !== BasePensionType.NewSystemMedicalAndRehabilitation
) {
methods.setValue(
'typeOfBasePension',
BasePensionType.NewSystemDisability,
)
}
} else {
if (
- typeOfBasePension === BasePensionType.NewSystemDisability ||
- typeOfBasePension === BasePensionType.NewSystemPartialDisability ||
- typeOfBasePension === BasePensionType.NewSystemMedicalAndRehabilitation
+ currentType === BasePensionType.NewSystemDisability ||
+ currentType === BasePensionType.NewSystemPartialDisability ||
+ currentType === BasePensionType.NewSystemMedicalAndRehabilitation
) {
methods.setValue('typeOfBasePension', BasePensionType.Retirement)
}
}
- }, [isNewSystemActive, methods, typeOfBasePension])
+ }, [isNewSystemActive, methods])
6d8767e
to
7c52554
Compare
Pension Calculator - "After September 1 2025" year option
TODO: form2025PreviewMainTitle is a new contentful translation string (run extract-strings script)
What
Screenshots / Gifs
Screen.Recording.2024-12-06.at.14.55.38.mov
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements
Documentation