-
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(old-age-pension): Show conclusion page based on income plan status #17087
Conversation
WalkthroughThe changes in this pull request introduce a new API export for the Social Insurance Administration module, specifically 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17087 +/- ##
==========================================
+ Coverage 35.74% 35.82% +0.08%
==========================================
Files 6925 6936 +11
Lines 147559 147519 -40
Branches 42010 42262 +252
==========================================
+ Hits 52745 52854 +109
+ Misses 94814 94665 -149
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 336 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
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: 2
🧹 Outside diff range and nitpick comments (7)
libs/application/templates/social-insurance-administration/old-age-pension/src/dataProviders/index.ts (1)
38-42
: Add JSDoc documentation for the new API export.Consider adding JSDoc documentation to describe the purpose and return type of this API, which will improve maintainability and help other developers understand its usage.
+/** + * Retrieves the latest income plan for the applicant from the Social Insurance Administration. + * Used to determine the conclusion page display in the old-age pension application. + */ export const SocialInsuranceAdministrationLatestIncomePlan = defineTemplateApi({ action: 'getLatestIncomePlan', externalDataId: 'socialInsuranceAdministrationLatestIncomePlan', namespace: 'SocialInsuranceAdministration', })libs/application/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx (2)
21-30
: Add error handling for external data fetching.While the code works, it would be more robust to handle potential errors when fetching external data. Consider adding error boundaries or fallback UI for cases where
application.externalData
might be undefined.- const { hasIncomePlanStatus } = getApplicationExternalData( - application.externalData, - ) + const { hasIncomePlanStatus } = getApplicationExternalData( + application.externalData ?? {} + )
48-87
: Reduce code duplication in conditional rendering.The AccordionCard and its contents are largely duplicated between the two conditions. Consider extracting the common UI elements into a separate component.
const ConclusionCard: FC<{ title: string }> = ({ title }) => ( <AccordionCard id="conclusion-card" label={title} startExpanded={true} > <Box marginBottom={4}> <Markdown> {formatMessage( oldAgePensionFormMessage.conclusionScreen.nextStepsText, )} </Markdown> </Box> <BulletList space="gutter" type="ul"> <Markdown> {formatMessage( oldAgePensionFormMessage.conclusionScreen.bulletList, )} </Markdown> </BulletList> </AccordionCard> )Also applies to: 103-138
libs/application/templates/social-insurance-administration/old-age-pension/src/forms/Prerequisites.ts (1)
167-170
: Consider adding configuration options for better reusability.The data provider is added correctly, but unlike some other providers (e.g., UserProfileApi), it lacks configuration options that might be useful for reuse in different contexts.
Consider adding relevant configuration options like:
buildDataProviderItem({ provider: SocialInsuranceAdministrationLatestIncomePlan, title: '', + subTitle: oldAgePensionFormMessage.pre.incomePlanDataDescription, + loadingMessage: oldAgePensionFormMessage.pre.loadingIncomePlan, + failureMessage: oldAgePensionFormMessage.pre.failedToLoadIncomePlan, }),libs/application/templates/social-insurance-administration/old-age-pension/src/lib/oldAgePensionUtils.ts (2)
257-260
: Consider adding JSDoc documentation for the new field.The implementation looks good and follows the existing pattern. Consider adding JSDoc documentation to describe the purpose and possible values of
hasIncomePlanStatus
.+/** Status of the latest income plan for the applicant */ const hasIncomePlanStatus = getValueViaPath( externalData, 'socialInsuranceAdministrationLatestIncomePlan.data.status', ) as string
257-260
: Consider using a more specific type for income plan status.Instead of using a generic string type, consider defining an enum or union type for the possible status values. This would provide better type safety and self-documentation.
type IncomePlanStatus = 'PENDING' | 'APPROVED' | 'REJECTED' | 'NOT_FOUND'; const hasIncomePlanStatus = getValueViaPath( externalData, 'socialInsuranceAdministrationLatestIncomePlan.data.status', ) as IncomePlanStatusAlso applies to: 276-276
libs/application/templates/social-insurance-administration/old-age-pension/src/lib/messages.ts (1)
264-275
: LGTM! Consider enhancing the message descriptions.The new messages are well-structured and properly internationalized. However, the descriptions could be more detailed to provide better context for translators.
Consider updating the descriptions to be more specific:
title: { id: 'oap.application:conclusionScreen.expandableDescriptionField.title', defaultMessage: 'Hvað gerist næst?', - description: 'What happens next', + description: 'Title for the section explaining next steps after submitting the pension application', }, alertMessage: { id: 'oap.application:conclusionScreen.alertMessage', defaultMessage: 'Umsókn um ellilífeyri hefur verið send til Tryggingastofnunar', - description: - 'An application for old age pension has been sent to the Social Insurance Administration', + description: + 'Confirmation message shown to users after successfully submitting their old age pension application', },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
libs/application/templates/social-insurance-administration/old-age-pension/src/dataProviders/index.ts
(1 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx
(1 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/fields/index.ts
(1 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/forms/OldAgePensionForm.ts
(1 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/forms/Prerequisites.ts
(2 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/lib/OldAgePensionTemplate.ts
(2 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/lib/messages.ts
(1 hunks)libs/application/templates/social-insurance-administration/old-age-pension/src/lib/oldAgePensionUtils.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
libs/application/templates/social-insurance-administration/old-age-pension/src/dataProviders/index.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/old-age-pension/src/fields/index.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/social-insurance-administration/old-age-pension/src/forms/OldAgePensionForm.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/old-age-pension/src/forms/Prerequisites.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/old-age-pension/src/lib/OldAgePensionTemplate.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/old-age-pension/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."
libs/application/templates/social-insurance-administration/old-age-pension/src/lib/oldAgePensionUtils.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 (8)
libs/application/templates/social-insurance-administration/old-age-pension/src/fields/index.ts (2)
5-5
: LGTM! Export follows the established pattern.
The new export for Conclusion
uses named export syntax, which is consistent with most other exports in this file.
Line range hint 1-5
: Consider standardizing export patterns for better tree-shaking.
Two components use default exports while others use named exports. Consider converting EmployersOverview
and EmployersRatioMonthly
to named exports for consistency and better tree-shaking support.
Also, verify that any TypeScript types/interfaces from the Conclusion
component that might be needed by consumers are being exported.
libs/application/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx (2)
1-20
: LGTM! Well-organized imports following best practices.
The imports are properly structured and support effective tree-shaking. The TypeScript types are correctly imported from their respective modules.
89-139
: LGTM! Success state implementation is correct.
The implementation correctly handles the case when an income plan exists, showing appropriate success messages and linking to the service portal.
libs/application/templates/social-insurance-administration/old-age-pension/src/forms/Prerequisites.ts (1)
38-38
: Verify the reusability of the imported data provider.
Since this is in the libs directory, let's ensure the data provider is properly exported and reusable across different NextJS apps.
✅ Verification successful
The data provider is properly exported and reused across templates
The verification shows that SocialInsuranceAdministrationLatestIncomePlan
is:
- Properly exported from data providers in both old-age-pension and income-plan templates
- Follows the same implementation pattern using
defineTemplateApi
- Consistently used across multiple components within both templates
- Maintains consistent naming and configuration across usages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the data provider is properly exported and potentially reused
ast-grep --pattern 'export const SocialInsuranceAdministrationLatestIncomePlan'
# Check for other potential usages
rg "SocialInsuranceAdministrationLatestIncomePlan" --type ts
Length of output: 3280
libs/application/templates/social-insurance-administration/old-age-pension/src/lib/OldAgePensionTemplate.ts (2)
101-101
: LGTM! API integration follows established patterns
The SocialInsuranceAdministrationLatestIncomePlan
API is correctly integrated into the PREREQUISITES
state configuration, following the same pattern as other Social Insurance Administration APIs. This addition aligns with the PR objective to show the conclusion page based on income plan status.
41-41
: Verify the reusability of the imported API
The import follows TypeScript conventions and maintains proper organization. Let's verify its reusability across different NextJS apps.
✅ Verification successful
Let me gather more information about the API definition to ensure it's properly implemented.
The imported API is properly defined and reused across templates
The verification shows that SocialInsuranceAdministrationLatestIncomePlan
is:
- Correctly defined as a reusable template API with proper namespace and action
- Used consistently in both old-age pension and income plan templates
- Follows the same pattern as other social insurance administration APIs
- Properly imported and integrated in the Prerequisites forms
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the API is defined as a reusable module
# and verify its usage across the codebase
# Check API definition
ast-grep --pattern 'defineTemplateApi({
action: "getLatestIncomePlan",
$$$
})'
# Check for other potential usages
rg "SocialInsuranceAdministrationLatestIncomePlan" --type ts
Length of output: 1793
Script:
#!/bin/bash
# Check the API definition in both locations
fd "index.ts" --type f --exec cat {} \; | rg -A 10 "SocialInsuranceAdministrationLatestIncomePlan = defineTemplateApi"
Length of output: 1085
libs/application/templates/social-insurance-administration/old-age-pension/src/forms/OldAgePensionForm.ts (1)
790-815
: Implementation of the conclusionSection
is appropriate and aligns with coding guidelines
The addition of the conclusionSection
with dynamic title handling based on hasIncomePlanStatus
enhances the user experience. The use of buildSection
and buildMultiField
is consistent with the application's structure.
...ication/templates/social-insurance-administration/old-age-pension/src/dataProviders/index.ts
Show resolved
Hide resolved
...on/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx
Outdated
Show resolved
Hide resolved
...on/templates/social-insurance-administration/old-age-pension/src/fields/Conclusion/index.tsx
Outdated
Show resolved
Hide resolved
...ion/templates/social-insurance-administration/old-age-pension/src/forms/OldAgePensionForm.ts
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.
lgtm!
…us (#17087) * start changes to conclusion page * add component for dynamic conclusion page * format * amendments after review plus small text change * remove unused import * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> 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
Hs no income plan:
Has income plan:
Checklist:
Summary by CodeRabbit
New Features
Conclusion
component that dynamically displays messages based on income plan status.Bug Fixes