-
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(social-insurance-administration): update model #17302
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces a new optional field Changes
Sequence DiagramsequenceDiagram
participant Client
participant GraphQL
participant SocialInsuranceService
participant PaymentModel
Client->>GraphQL: Request Payment Plan
GraphQL->>SocialInsuranceService: getPaymentPlan()
SocialInsuranceService->>PaymentModel: Create Payment with markWithAsterisk
PaymentModel-->>SocialInsuranceService: Return Payment
SocialInsuranceService-->>GraphQL: Return Payment Plan
GraphQL-->>Client: Render Payment Plan with Asterisk
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (8)
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 #17302 +/- ##
==========================================
+ Coverage 35.68% 35.69% +0.01%
==========================================
Files 6931 6924 -7
Lines 148785 148613 -172
Branches 42509 42436 -73
==========================================
- Hits 53091 53052 -39
+ Misses 95694 95561 -133
Flags with carried forward coverage won't be shown. Click here to find out more. see 38 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 3 Total Test Services: 0 Failed, 3 Passed Test Services
|
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 comments (1)
libs/clients/social-insurance-administration/src/clientConfig.json (1)
Line range hint
2482-2502
: Ensure the formbuilder route remains properly secured.This block references a “FormBuilder” result set. Validate the endpoint’s access control to prevent unauthorized usage.
🧹 Nitpick comments (13)
libs/portals/my-pages/social-insurance-maintenance/src/screens/PaymentPlan/PaymentPlan.tsx (1)
90-92
: New temporary warning text
Displaying the added warning clearly underscores upcoming changes. Consider using a distinct style or icon if you want to emphasize it further.libs/portals/my-pages/core/src/components/ScrollableMiddleTable/ScrollableMiddleTable.tsx (1)
43-46
: Hardcoded column widths
Consider extracting these magic numbers into theme variables or a constants file for improved maintainability.libs/api/domains/social-insurance/src/lib/socialInsurance.service.ts (1)
101-101
: Add a comment clarifying the purpose of the markWithAsterisk field.The field helps indicate special or highlighted payments in the service logic, but it would be beneficial to briefly explain its usage in the code or via a comment so future maintainers understand when and why it’s set to true.
libs/clients/social-insurance-administration/src/clientConfig.json (10)
3-3
: Document the version update in release notes or changelog.You updated the API version from a previous value to
"v1.0"
. Make sure to note this change in your project documentation or release notes, allowing consumers to be aware of potential breaking changes.
Line range hint
478-581
: Confirm alignment of “Application” endpoint changes.These lines handle various application types via a single path parameter. The request body definition is generic; consider adding validation or references in the code to handle different application scenarios gracefully.
Line range hint
1302-1387
: Ensure year parameters use consistent integer format checks.You define an integer year parameter. Confirm user-supplied values are validated in the service code to prevent invalid data from traveling downstream.
Line range hint
1475-1495
: Double-check logic for 404 and 500 statuses in the income plan endpoint.Any extra states, such as 304 (Not Modified) or 409 (Conflict), might be relevant. Evaluate if you need additional status codes or if these cover all real error cases.
Line range hint
2043-2080
: Consider edge cases for the validyears array.You return an array of years. Validate empty arrays, large year inputs, or future/past constraints to prevent potential off-by-one or misaligned data.
2551-2552
: Add or update documentation for the new document type.You introduced or updated “type” and “content” for documents. Ensure external docs reflect these fields for integrators.
Line range hint
2559-2628
: Review income type property naming for consistency.Properties like
amountJan
,amountFeb
, etc., use a monthly breakdown. This is clear, but ensuring consistent usage across your domain can reduce confusion in the long run.
2646-2647
: Validate that “month” and “amount” are correct data types.The “amount” uses
"type": "number", "format": "double"
. If you need currency operations, consider further constraints or a decimal representation to avoid floating-point issues.Would you like help applying a decimal-based solution to mitigate floating-point inaccuracies?
2654-2655
: Ensure spouse date fields are clearly documented.The property “dateOfDeath” uses a string with
format: "date-time"
. Double-check any timezones or date handling in your back end.
3383-3387
: Enhance clarity of markWithAsterisk usage in the PaymentPlanRowDto schema.The property “markWithAsterisk” is introduced here. Include a short note in the schema description or docstrings to indicate usage guidelines for client integrations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/api/domains/social-insurance/src/lib/models/payments/payment.model.ts
(1 hunks)libs/api/domains/social-insurance/src/lib/socialInsurance.service.ts
(1 hunks)libs/clients/social-insurance-administration/src/clientConfig.json
(58 hunks)libs/portals/my-pages/core/src/components/ScrollableMiddleTable/ScrollableMiddleTable.tsx
(1 hunks)libs/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTable.graphql
(1 hunks)libs/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTableRow.tsx
(1 hunks)libs/portals/my-pages/social-insurance-maintenance/src/lib/messages.ts
(1 hunks)libs/portals/my-pages/social-insurance-maintenance/src/screens/PaymentPlan/PaymentPlan.tsx
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
libs/api/domains/social-insurance/src/lib/models/payments/payment.model.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/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTable.graphql (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/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTableRow.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/portals/my-pages/core/src/components/ScrollableMiddleTable/ScrollableMiddleTable.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/portals/my-pages/social-insurance-maintenance/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/portals/my-pages/social-insurance-maintenance/src/screens/PaymentPlan/PaymentPlan.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/api/domains/social-insurance/src/lib/socialInsurance.service.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/clients/social-insurance-administration/src/clientConfig.json (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 (28)
libs/portals/my-pages/social-insurance-maintenance/src/screens/PaymentPlan/PaymentPlan.tsx (4)
1-1
: Imports look consistent
All imported modules appear to be in use and appropriate for this component.
13-13
: New query import
Good addition using the dedicated generated query to retrieve previous payments.
19-19
: Query usage
Fetching data directly in the component is straightforward and seems well-handled (loading and error states are defined).
79-79
: Footer message rendering
This line modifies existing footer text concatenation. No issues found.libs/api/domains/social-insurance/src/lib/models/payments/payment.model.ts (1)
12-14
: NewmarkWithAsterisk
field
This boolean field helps highlight special payment conditions. Implementation looks good.libs/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTableRow.tsx (1)
64-66
: Conditional asterisk handling
Appending the asterisk ifmarkWithAsterisk
is true effectively signals any special note on payments.libs/portals/my-pages/social-insurance-maintenance/src/lib/messages.ts (1)
21-25
: New message added
ThemaintenanceFooterTemporaryWarning
message is clearly labeled, improves user awareness, and is well-structured for localization.libs/portals/my-pages/social-insurance-maintenance/src/components/PaymentGroupTable/PaymentGroupTable.graphql (1)
14-14
: Ensure front-end rendering logic agrees with the new markWithAsterisk field.The added field is retrieved and presumably used in other components for conditional display of an asterisk. Verify that all UI elements referencing this new field are tested to avoid rendering regressions.
libs/clients/social-insurance-administration/src/clientConfig.json (20)
15-15
: Confirm that the schema type is correct for all paths consuming the applicationType.You’ve asserted
"type": "string"
for theapplicationType
path parameter. Verify this logic aligns with the actual backend usage and the domain model.
80-80
: Use consistent HTTP status descriptions throughout.Changing
"description": "Server Error"
to"description": "Internal Server Error"
is good for clarity and uniformity. Ensure the same approach is applied across all endpoints.
100-100
: Likewise, standardize the “OK” status description.Renaming
"Success"
to"OK"
follows typical HTTP conventions. The changes appear correct here.
Line range hint
188-208
: Maintain consistent status labeling across endpoints.Lines updating
"description": "Internal Server Error"
and"description": "OK"
for these endpoints are consistent with lines 80 and 100. Good job keeping the status descriptions uniform.
230-247
: Validate the new path and request body references.Introducing
"/api/protected/v1/Applicant/onetimepayment"
and referencing"TrWeb.Commons.ExternalPortals.Api.Models.FormBuilder.Application"
is logical. Confirm that other services or references pointing to the old path are updated to avoid routes being broken.
356-477
: Check the new /api/protected/v1/Applicant/balance endpoint thoroughly.This block defines a new endpoint for retrieving the estate balance. Ensure it is covered by integration tests to confirm correct request/response handling.
613-613
: Validate user input for "applicationGuid".Ensure the string type for the parameter meets any length or format constraints, e.g., UUID if that applies.
Line range hint
708-727
: Added response codes: check error handling flows.Switching from “Server Error” to “Internal Server Error” and including a “204 No Content” response clarifies outcomes. Confirm that downstream consumers handle these new status codes properly.
846-851
: Use consistent schema for path parameters and query parameters.The lines define parameters for document retrieval. Ensure
"schema": { "type": "string" }
fits real usage constraints (e.g., lengths, potential enumerations).
Line range hint
916-936
: Keep success/error responses alike across endpoints.Similar to other 200/500 updates, lines 916 and 936 unify the usage of “Internal Server Error” and “OK.” Keep verifying no code references an outdated description.
Line range hint
1603-1623
: Temporary calculations ensure consistent type usage.You reference
PaymentPlan.PaymentPlanDto
for the 200 response. Verify that the new markWithAsterisk field (if relevant) is also recognized here, or confirm that these calculations do not require it.
Line range hint
1711-1731
: Provide robust testing for newly introduced conditions.The lines define an “IncomePlanConditionsDto” with a 200 response. Make sure your tests confirm the property
showTemporaryCalculations
is returned when expected.
Line range hint
1762-1847
: Verify the new query param for PaymentPlan retrieval.You’ve introduced a
year
query param. Confirm that the code reading this param is validated and tested.
Line range hint
1935-1955
: Double-check naming consistency of legitimatepayments resource.The resource name matches the front-end usage. If used in multiple places, ensure no mismatched references remain.
[approve]
Line range hint
2174-2194
: Confirm pension calculation changes do not conflict with the rest of the application.Changes associated with “Internal Server Error” and “OK” statuses ensure clarity. Validate that client libraries handle any newly introduced fields or assumptions about the pension calculations.
Line range hint
2291-2411
: Expanded test routes: ensure user acceptance testing.“/api/protected/v1/Test/hello” and “/api/protected/v1/Test” appear to be test endpoints. Confirm they remain or remove them in production to avoid confusion or security exposures.
2530-2537
: Confirm coverage for new bank field properties.The updated object includes additional fields like “foreignBankAddress.” Verify that all necessary logic to handle these is in place.
2544-2544
: Check references to CreateApplicationFromPaperReturn.Ensure any calls to generate or parse these returns are tested, especially if you’ve changed the object structure.
Line range hint
3250-3278
: Confirm withholding tax data alignment.The monthly fields (e.g., january, february) are typed as doubles. Validate that derived calculations remain correct for partial amounts or rounding concerns.
Line range hint
3407-3497
: Pension calculator fields might interact with the new asterisk logic.While not directly referencing
markWithAsterisk
, confirm that the newly introduced logic doesn’t conflict with the pension calculation.
* fix: update * fix: use asterisk * fix: add asterisk for some values* * chore: add message
What
Checklist:
Summary by CodeRabbit
New Features
markWithAsterisk
field to Payment modelDocumentation
UI Changes
API Updates
markWithAsterisk
field