Skip to content
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

#2001 - Changes to handle PD PPD Part 1 #2115

Merged
merged 11 commits into from
Jul 18, 2023

Conversation

dheepak-aot
Copy link
Collaborator

@dheepak-aot dheepak-aot commented Jul 14, 2023

System changes to handle PD PPD

  • Created DB migration to add new type permanent_disability_status and add column pd_status to student table. Updated the entity model and created enum PDStatus.
  • Updated SIMS API, and Vue with PDStatus enum. (By doing this, the PD status field is handled on both student profile and student application)

Student Profile:
image

Student Application:
image

  • The existing pd_verified column is kept as is to avoid workflow changes as per the story.

  • Student profile form definition updated to check for value Not requested instead of Not Requested to display the Apply for PD Status button

  • When "apply to PD status" is not allowed due to SIN validation is not complete or already applied for PD status, code changes are made to display the appropriate error message to the toast.

To be done in upcoming PR

  • Read the PD response from ATBC and update the pd status. Remove/Refactor the existing code.
  • Remove refactor existing code
  • E2E Tests

@dheepak-aot dheepak-aot self-assigned this Jul 14, 2023
@dheepak-aot dheepak-aot added SIMS-Api SIMS-Api Web Portal Form.io Form IO definitions changed. DB DB migration involved labels Jul 14, 2023
ALTER TABLE
sims.students
ADD
COLUMN IF NOT EXISTS pd_status sims.permanent_disability_status;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope that it will not open a can of worms about naming possibilities 😉
I believe that the acronyms for PD/PPD mean

  • PPD: Persistent or Prolonged Disability
  • PD: Permanent Disability
    With the above in mind I believe that instead of permanent_disability_status it should be only disability_status.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussion with the dev team, we decided to use disability status instead of permanent disability status and try to change the existing code as much as possible.

@@ -122,3 +122,8 @@ export const DUPLICATE_SABC_CODE = "DUPLICATE_SABC_CODE";
* Institution location not valid.
*/
export const INSTITUTION_LOCATION_NOT_VALID = "INSTITUTION_LOCATION_NOT_VALID";

/**
* Request for permanent disability not allowed.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requests are no longer specifically for permanent disability.

) {
throw new UnprocessableEntityException(
"Either the client does not have a validated SIN or the request was already sent to ATBC.",
new ApiProcessError(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

) {
throw new UnprocessableEntityException(
"Either the client does not have a validated SIN or the request was already sent to ATBC.",
new ApiProcessError(
"Either SIN validation is not complete or requested for PD already.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that we can have it adjusted to PD/PPD for now.

@@ -0,0 +1,10 @@
/**
* Permanent disability status of student.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is SIMS used to be named using PD as permanent disability which is no longer true with the inclusion of PPD.
I would say that we need to reinforce it for the new things and try to adjust the current ones as possible.
As a suggestion, I would remove the "Permanent" from the comment and name the below enum as DisabilityStatus.

snackBar.error(
"An error happened during the apply PD process. Please try after sometime.",
"Unexpected error while applying for PD. Please try after sometime.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change PD to PD/PPD?

@@ -306,7 +306,7 @@ describe("ApplicationOfferingChangeRequestInstitutionsController(e2e)-getComplet

const searchKeyword = "TestStudent";
const endpoint = `/institutions/location/${collegeFLocation.id}/application-offering-change-request/completed?page=0&pageLimit=10&searchCriteria=${searchKeyword}`;
console.log(endpoint);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -62,7 +62,7 @@ describe("StudentInstitutionsController(e2e)-getStudentProfile", () => {
},
phone: student.contactInfo.phone,
},
pdStatus: determinePDStatus(student),
pdStatus: student.pdStatus,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// studentPDVerified is true if PD confirmed by ATBC or is true from sfas_individual table.
// studentPDVerified is false if PD denied by ATBC.
// To apply for a PD, SIN validation must be completed for the student and
// not applied for PD already.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dheepak-aot , can you confirm this? In our grooming, I remember saying that the student can apply for PD multiple times, especially for the PPD

Copy link
Collaborator Author

@dheepak-aot dheepak-aot Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bring this up.
I had a quick chat with @JasonCTang on the same and got to know that, as per the current understanding, student requests disability status only once in our application. In case they want to apply again because of something happened to them later after losing their status, they update ATBC directly and ATBC will be updating the student disability status which we read on daily basis through their response API.
I will update this information to the ticket.

Let me know if it is still not clear. We can talk.

Copy link
Contributor

@ann-aot ann-aot Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dheepak-aot for the details. Ya, adding it to the ticket will make it easier for future development. Keeping it open for other devs

Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments

AND pd_date_sent IS NOT NULL THEN 'Requested'
WHEN pd_verified = true THEN 'PD'
WHEN pd_verified = false THEN 'Declined'
ELSE 'Not requested' :: sims.disability_status
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get this update is written for the existing data in Test, but is there a chance that any PDs can be PPDs? are we considering any update in the existing data update from ATBC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no PPD in our system so far so we cannot do anything in the query about it.

If a student is PPD, ATBC response will be read and updated in our system.

auditUser: User,
): Promise<UpdateResult> {
const now = new Date();
return this.repo.update(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link
Collaborator

@sh16011993 sh16011993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @dheepak-aot 👍

@@ -15,6 +15,8 @@ export class CreateStudentAPIInDTO extends AddressDetailsFormAPIDTO {
sinNumber: string;
@Expose()
gender: string;
@Expose()
sinConsent: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @dheepak-aot. thanks for doing the changes, wava fix as well as the label changes 👍

@github-actions
Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 17.86% ( 2129 / 11922 )
Methods: 8.3% ( 126 / 1518 )
Lines: 20.64% ( 1865 / 9034 )
Branches: 10.07% ( 138 / 1370 )

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 49.81% ( 267 / 536 )
Methods: 41.56% ( 32 / 77 )
Lines: 55.33% ( 218 / 394 )
Branches: 26.15% ( 17 / 65 )

@github-actions
Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 71% ( 399 / 562 )
Methods: 61.97% ( 44 / 71 )
Lines: 72.97% ( 351 / 481 )
Branches: 40% ( 4 / 10 )

@github-actions
Copy link

E2E SIMS API Coverage Report

Totals Coverage
Statements: 49.76% ( 3536 / 7106 )
Methods: 45.06% ( 415 / 921 )
Lines: 54.97% ( 2914 / 5301 )
Branches: 23.42% ( 207 / 884 )

Copy link
Collaborator

@guru-aot guru-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Nice work @dheepak-aot

@dheepak-aot dheepak-aot merged commit 69013fa into main Jul 18, 2023
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:44 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:45 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:45 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:45 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:45 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:47 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV July 18, 2023 17:47 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot deleted the feature/#2001-enable-sims-ppd-part-1 branch July 18, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB DB migration involved Form.io Form IO definitions changed. SIMS-Api SIMS-Api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants