-
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
fix(inheritance-report): Prepaid finishups #15323
Conversation
WalkthroughThe changes introduce new fields for applicants, executors, and spouses in the inheritance report application. The handling of assets in the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 (1)
libs/application/templates/inheritance-report/src/lib/utils/helpers.ts (1)
Line range hint
122-122
: Consider using optional chaining for better safety.To enhance code safety and readability, consider using optional chaining when accessing nested properties.
- const estateData = (application.externalData.syslumennOnEntry?.data as { + const estateData = (application.externalData.syslumennOnEntry?.data as? {
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (13)
- libs/application/template-api-modules/src/lib/modules/templates/inheritance-report/utils/mappers.ts (1 hunks)
- libs/application/templates/inheritance-report/src/fields/Overview/OverviewAssets/index.tsx (2 hunks)
- libs/application/templates/inheritance-report/src/fields/Overview/OverviewHeirs/index.tsx (1 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/applicant.ts (1 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/assets.ts (10 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/deceased.ts (1 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/applicant.ts (3 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/dataCollection.ts (1 hunks)
- libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/inheritance.ts (2 hunks)
- libs/application/templates/inheritance-report/src/lib/constants.ts (1 hunks)
- libs/application/templates/inheritance-report/src/lib/dataSchema.ts (3 hunks)
- libs/application/templates/inheritance-report/src/lib/messages.ts (1 hunks)
- libs/application/templates/inheritance-report/src/lib/utils/helpers.ts (2 hunks)
Files skipped from review due to trivial changes (4)
- libs/application/templates/inheritance-report/src/fields/Overview/OverviewHeirs/index.tsx
- libs/application/templates/inheritance-report/src/forms/sections/deceased.ts
- libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/applicant.ts
- libs/application/templates/inheritance-report/src/lib/messages.ts
Additional context used
Path-based instructions (9)
libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/inheritance.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/inheritance-report/src/forms/sections/prepaidInheritance/dataCollection.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/inheritance-report/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/inheritance-report/src/forms/sections/applicant.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/inheritance-report/src/lib/utils/helpers.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/inheritance-report/src/fields/Overview/OverviewAssets/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/template-api-modules/src/lib/modules/templates/inheritance-report/utils/mappers.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/inheritance-report/src/lib/dataSchema.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/inheritance-report/src/forms/sections/assets.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."
Biome
libs/application/templates/inheritance-report/src/lib/utils/helpers.ts
[error] 122-122: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
libs/application/templates/inheritance-report/src/lib/dataSchema.ts
[error] 169-171: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 181-183: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 560-561: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 88-88: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
Additional comments not posted (11)
libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/inheritance.ts (1)
8-8
: Ensure consistent import paths.The import statement for
PrePaidInheritanceOptions
uses a relative path. Confirm if this aligns with the project's standards for import paths, especially for constants that might be used across various modules.libs/application/templates/inheritance-report/src/forms/sections/prepaidInheritance/dataCollection.ts (1)
35-36
: Localized titles and subtitles are correctly implemented.The use of localized strings for the titles and subtitles ensures that the application supports internationalization. This change aligns with best practices for global applications.
libs/application/templates/inheritance-report/src/lib/constants.ts (1)
23-28
: Enum values are correctly defined with string assignments.Defining enum values as strings provides clarity and can be beneficial for debugging and logging. This is a good practice, especially when these values are used in UI components or are sent to/from external APIs.
libs/application/templates/inheritance-report/src/lib/utils/helpers.ts (1)
57-78
: Refactored asset inclusion logic looks good!The consolidation of asset inclusion logic into a single function using boolean flags derived from
isPrePaid
and selected options enhances readability and maintainability.libs/application/templates/inheritance-report/src/fields/Overview/OverviewAssets/index.tsx (1)
36-37
: Streamlined logic for asset inclusion is effective and maintainable.The refactoring to use a consolidated approach for determining asset sections based on
isPrePaid
andanswers
enhances both the clarity and maintainability of the code.Also applies to: 42-45, 47-51, 126-129, 131-135, 140-142, 149-152, 154-158
libs/application/template-api-modules/src/lib/modules/templates/inheritance-report/utils/mappers.ts (1)
104-117
: Addition of new fields inexpandAnswers
function is well-implemented.The inclusion of
prePaidApplicant
,prepaidInheritance
,name
, andnationalId
fields forexecutor
andspouse
objects enhances the data handling capabilities of the module.libs/application/templates/inheritance-report/src/lib/dataSchema.ts (1)
Line range hint
137-198
: Schema enhancements for handling prepaid inheritance are well-defined.The additions and modifications to the schema to include
prePaidApplicant
andprepaidInheritance
options are well-implemented, enhancing both the robustness and functionality of the module.libs/application/templates/inheritance-report/src/forms/sections/assets.ts (4)
8-8
: Ensure consistent import usage.The addition of
getValueViaPath
is used in the conditional logic for asset sections. This is a good use of utility functions to simplify the retrieval of deeply nested properties.
16-20
: Confirm the import of constants.The import of
PrePaidInheritanceOptions
is introduced, which is used for checking asset types in conditions. This is a correct and necessary change for the implemented logic.
114-114
: Consistent conditional logic across sections.The conditions for showing various asset sections based on
PREPAID_INHERITANCE
are implemented consistently. This ensures that the form adapts correctly based on the type of inheritance being processed.Also applies to: 176-176, 243-243, 305-305, 390-390
448-451
: Validate inclusion logic for specific asset types.The conditions for including specific asset types like stocks, money, and other assets are correctly using the
PrePaidInheritanceOptions
to check against theprepaidInheritance
answers. This is a robust way to dynamically include sections based on form responses.Also applies to: 536-539, 620-623
libs/application/templates/inheritance-report/src/forms/sections/assets.ts
Show resolved
Hide resolved
...pplication/templates/inheritance-report/src/forms/sections/prepaidInheritance/inheritance.ts
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 14 Total Test Services: 0 Failed, 13 Passed Test ServicesThis report shows up to 10 services
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15323 +/- ##
==========================================
- Coverage 37.07% 37.07% -0.01%
==========================================
Files 6508 6508
Lines 132437 132440 +3
Branches 37874 37878 +4
==========================================
Hits 49102 49102
- Misses 83335 83338 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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
Checklist:
Summary by CodeRabbit
New Features
prePaidApplicant
,prepaidInheritance
,name
,nationalId
) for improved reporting of executor and spouse details.Refactor
OverviewHeirs
component for better layout consistency.prePaidApplicant
for naming consistency.Bug Fixes
Documentation
Chores