-
Notifications
You must be signed in to change notification settings - Fork 14
#4517 - Bypass partner declaration exception (DV) #4651
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
Conversation
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.
Pull Request Overview
This PR introduces a new relationship status ("marriedUnable") to support partner declaration exceptions and aligns backend logic to incorporate this change. Key updates include:
- Expanding type definitions and enums to include "marriedUnable".
- Updating conversion functions in the utility module to map both Married and MarriedUnable to the same codes.
- Adding a new DB migration to insert the new status into the database.
Reviewed Changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.
File | Description |
---|---|
sources/packages/backend/workflow/test/models/assessment.model.ts | Updated the RelationshipStatusType union to include "marriedUnable". |
sources/packages/backend/libs/utilities/src/esdc-utils.ts | Modified conversion functions (getMaritalStatusCode and getPartTimeMaritalStatusCode) to include a case for "MarriedUnable". |
sources/packages/backend/libs/sims-db/src/entities/relationship-status.type.ts | Added a new enum entry for MarriedUnable with proper documentation. |
sources/packages/backend/apps/db-migrations/src/migrations/1745962287131-AddRelationshipStatusMarriedUnable.ts | Introduced a migration for adding the new relationship status. |
Files not reviewed (4)
- sources/packages/backend/apps/db-migrations/src/sql/Types/Add-relationship-status-married-unable.sql: Language not supported
- sources/packages/backend/apps/db-migrations/src/sql/Types/Rollback-add-relationship-status-married-unable.sql: Language not supported
- sources/packages/forms/src/form-definitions/studentexceptions.json: Language not supported
- sources/packages/web/src/assets/css/formio-shared.scss: Language not supported
Comments suppressed due to low confidence (2)
sources/packages/backend/libs/utilities/src/esdc-utils.ts:17
- Consider adding tests to verify that both RelationshipStatus.Married and RelationshipStatus.MarriedUnable correctly map to the expected marital status code for the MSFAA request file.
switch (maritalStatus) {
sources/packages/backend/libs/utilities/src/esdc-utils.ts:62
- Consider adding tests to ensure getPartTimeMaritalStatusCode handles RelationshipStatus.MarriedUnable in the same way as RelationshipStatus.Married, returning the correct status code.
switch (maritalStatus) {
...ackend/apps/db-migrations/src/migrations/1745962287131-AddRelationshipStatusMarriedUnable.ts
Show resolved
Hide resolved
'married', | ||
'single', | ||
'other', | ||
'marriedUnable' |
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.
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.
In general, we do not use dropdown values with white spaces, doing this for these seems against what we did for others.
Business can have an opinion about how it can potentially be displayed to the user, but how we persist will be a technical decision.
My understanding on the "provided AC value" was a suggestion on how to shorten the pretty long option.
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.
I asked the same as this information goes raw to SDPR response as well.
Also there is an IER flag hasPartner which is based on marital status. Does it need to be taken care later? or should we follow the same path like other integrations here?
Line 207 in b73b424
const hasPartner = |
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.
It does not seem to be in the scope or planned for this ticket, but it would be worth asking.
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.
Business updated the AC to state: marriedUnable
.
@@ -12465,7 +13177,7 @@ | |||
}, | |||
"properties": {}, | |||
"calculateServer": true, | |||
"calculateValue": "// If the intensity is Part-time, student is always considered 'independant'.\r\nif (data.howWillYouBeAttendingTheProgram === \"Part Time\") {\r\n instance.setValue(\"independant\");\r\n}\r\n// If the intensity is Full-time, dependant status is calculated based on personal information answers.\r\nelse if (data.howWillYouBeAttendingTheProgram === \"Full Time\") {\r\n const isStudentIndependant =\r\n data.hasDependents === \"yes\" ||\r\n [\"married\", \"other\"].includes(data.relationshipStatus) ||\r\n data.outOfHighSchoolFor4Years === \"yes\" ||\r\n data.fulltimelabourForce === \"yes\" ||\r\n data.parentsDeceased === \"yes\" ||\r\n data.estranged === \"yes\" ||\r\n data.custodyOfChildWelfare === \"yes\";\r\n // If the student is not independant, dependant status is calculated based on personal information answer inputs.\r\n if (!isStudentIndependant) {\r\n const isPersonalInfoAnswered =\r\n !!data.hasDependents &&\r\n !!data.relationshipStatus &&\r\n !!data.outOfHighSchoolFor4Years &&\r\n !!data.fulltimelabourForce &&\r\n !!data.parentsDeceased &&\r\n !!data.estranged &&\r\n !!data.youthInCare &&\r\n (data.youthInCare === \"no\" || !!data.custodyOfChildWelfare);\r\n // If the answers to required personal information inputs are not provided, set the dependant status to null.\r\n // Otherwise, dependant status is calculated based on personal information answers.\r\n const calculatedStatusFromPersonalInfo = isPersonalInfoAnswered\r\n ? \"dependant\"\r\n : null;\r\n instance.setValue(calculatedStatusFromPersonalInfo);\r\n } else {\r\n instance.setValue(\"independant\");\r\n }\r\n}\r\n// If the intensity is not provided, set the value to null.\r\nelse {\r\n instance.setValue(null);\r\n}\r\n ", | |||
"calculateValue": "// If the intensity is Part-time, student is always considered 'independant'.\r\nif (data.howWillYouBeAttendingTheProgram === \"Part Time\") {\r\n instance.setValue(\"independant\");\r\n}\r\n// If the intensity is Full-time, dependant status is calculated based on personal information answers.\r\nelse if (data.howWillYouBeAttendingTheProgram === \"Full Time\") {\r\n const isStudentIndependant =\r\n data.hasDependents === \"yes\" ||\r\n [\"married\", \"other\", \"marriedUnable\"].includes(data.relationshipStatus) ||\r\n data.outOfHighSchoolFor4Years === \"yes\" ||\r\n data.fulltimelabourForce === \"yes\" ||\r\n data.parentsDeceased === \"yes\" ||\r\n data.estranged === \"yes\" ||\r\n data.custodyOfChildWelfare === \"yes\";\r\n // If the student is not independant, dependant status is calculated based on personal information answer inputs.\r\n if (!isStudentIndependant) {\r\n const isPersonalInfoAnswered =\r\n !!data.hasDependents &&\r\n !!data.relationshipStatus &&\r\n !!data.outOfHighSchoolFor4Years &&\r\n !!data.fulltimelabourForce &&\r\n !!data.parentsDeceased &&\r\n !!data.estranged &&\r\n !!data.youthInCare &&\r\n (data.youthInCare === \"no\" || !!data.custodyOfChildWelfare);\r\n // If the answers to required personal information inputs are not provided, set the dependant status to null.\r\n // Otherwise, dependant status is calculated based on personal information answers.\r\n const calculatedStatusFromPersonalInfo = isPersonalInfoAnswered\r\n ? \"dependant\"\r\n : null;\r\n instance.setValue(calculatedStatusFromPersonalInfo);\r\n } else {\r\n instance.setValue(\"independant\");\r\n }\r\n}\r\n// If the intensity is not provided, set the value to null.\r\nelse {\r\n instance.setValue(null);\r\n}\r\n", |
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.
👍
.formio-component-weight-bold > div:first-child { | ||
font-weight: $font-weight-bold; | ||
} |
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.
What is the purpose of this particular css?
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.
Apply a bold text to a component without interfering with other component items, for instance, error validators.
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.
I will add a comment.
@@ -457,6 +499,7 @@ | |||
|
|||
.form-check-input { | |||
position: inherit; | |||
vertical-align: top; |
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.
👍
Great Job! One minor comment and a clarification. |
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.
Looks good. Thanks for the clarifications. 👍
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.
Pull Request Overview
This PR implements the bypass partner declaration exception by introducing a new enum value "marriedUnable". Changes include updating the related type definitions, conversion logic in the backend utility functions, and adding a new database migration for the enum update.
- Updated RelationshipStatusType and RelationshipStatus enum to include "marriedUnable".
- Modified the marital status conversion functions in esdc-utils to support the new option.
- Added a new migration for updating the database with the "marriedUnable" option.
Reviewed Changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
sources/packages/backend/workflow/test/models/assessment.model.ts | Extended the RelationshipStatusType to include "marriedUnable". |
sources/packages/backend/libs/utilities/src/esdc-utils.ts | Updated conversion functions (MSFAA and Part-Time) to group Married and MarriedUnable statuses. |
sources/packages/backend/libs/sims-db/src/entities/relationship-status.type.ts | Added the MarriedUnable enum with accompanying documentation. |
sources/packages/backend/apps/db-migrations/src/migrations/1745962287131-AddRelationshipStatusMarriedUnable.ts | Created a migration for the new MarriedUnable enum value. |
Files not reviewed (4)
- sources/packages/backend/apps/db-migrations/src/sql/Types/Add-relationship-status-married-unable.sql: Language not supported
- sources/packages/backend/apps/db-migrations/src/sql/Types/Rollback-add-relationship-status-married-unable.sql: Language not supported
- sources/packages/forms/src/form-definitions/studentexceptions.json: Language not supported
- sources/packages/web/src/assets/css/formio-shared.scss: Language not supported
Comments suppressed due to low confidence (1)
sources/packages/backend/workflow/test/models/assessment.model.ts:51
- Ensure that the new 'marriedUnable' enum value is consistently handled in all partner information and income logic across the codebase.
| "marriedUnable";
|
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.
Great work @andrewsignori-aot 👍
…#4663) Same changes done for full-time in the previous PR (#4651) with a minor change for the full-time upload dir property that is saved to the DB.  
marriedUnable
option.Married
,file-upload-info-container
.Full-time Student application
Some styles were adjusted to ensure the validation message followed the others.
Ministry New Exception
TODO
marriedUnable
option.Migration Revert
Removed Citizenship for permanent residency
"Citizenship for permanent residency" was removed as part of #4662