#1683 - Completed application - E2E tests for application tracker #1832

andrewsignori-aot · 2023-03-23T02:56:34Z

Fix for PIR decline throwing an error while completing the process.
Refactor application progress details to get rid of the Sonarcloud issue.
Icon spacing adjusted (@hellolynn-tbtb is aware of and approved it).
Renamed factory method saveFakeApplicationCOE to saveFakeApplicationDisbursements allowing it to be used for these E2E tests also.
Removed prefix ${ClientTypeBaseRoute.Institution} from some E2E that were wrongly added.
E2E tests for the new endpoint added for the completed status.
ApplicationStudentsController(e2e)-getCompletedApplicationDetails
✓ Should throw NotFoundException when the application is not associated with the authenticated student. (390 ms)
✓ Should throw NotFoundException when the application is not in 'Completed' status. (75 ms)
✓ Should get application details when application is in 'Completed' status. (76 ms)
✓ Should get application details with scholastic standing change type when application has a scholastic standing 'Student did not complete program associated with. (60 ms)
✓ Should get application details with the most updated appeal status when the application has more than one student appeals associated with. (78 ms)

sources/packages/backend/apps/api/src/services/application/application.service.ts

ann-aot · 2023-03-24T18:34:34Z

sources/packages/backend/libs/test-utils/src/factories/application.ts

+    fakeOriginalAssessment,
+  );
+  savedApplication.currentAssessment = savedOriginalAssessment;
+  savedApplication.applicationStatus = ApplicationStatus.Enrolment;


mm, we are already saving the status at line 95 right? then why this assignment

I do not see a reason to. I will remove the last one.

sources/packages/backend/libs/test-utils/src/factories/application.ts

ann-aot · 2023-03-24T18:45:18Z

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

+
+  it("Should get application details when application is in 'Completed' status.", async () => {
+    // Arrange
+    const application = await saveFakeApplicationDisbursements(


mm, instead of getting a fake disbursement application and changing the data in the test to make it completed, can't we create a fakecompletedAppliaction? is there any reason for it?

When we need an application with disbursements we will need: createFakeApplication, createFakeStudentAssessment, createFakeDisbursementSchedule, createFakeEducationProgramOffering, createFakeUser, createFakeStudent, etc., and then have all of them saved properly. When this method was introduced in the past for COE (and now renamed/moved for disbursements), the idea was to concentrate all the complexity of calling the previously mentioned createFake methods and coordinate the way that they must be saved.
Does it answer? If not maybe I am not getting the point and we can have a chat. Please let me know.

I guess that the answer is the same from #1832 (comment)

sources/packages/backend/libs/test-utils/src/factories/student-scholastic-standing.ts

ann-aot · 2023-03-24T18:54:22Z

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

+    const scholasticStanding = createFakeStudentScholasticStanding({
+      submittedBy: submittedByInstitutionUser,
+    });
+    // 'Student did not complete program' is the only'scholastic standing that does not generate an assessment.


only scholastic

ann-aot · 2023-03-24T18:58:52Z

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

+    firstDisbursement.coeStatus = COEStatus.completed;
+    await disbursementScheduleRepo.save(firstDisbursement);
+    // Create a scholastic standing and have it associated with the completed application.
+    const scholasticStanding = createFakeStudentScholasticStanding({


application can be passed here itself right?

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

ann-aot · 2023-03-24T19:25:28Z

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

+      application.currentAssessment.disbursementSchedules;
+    firstDisbursement.coeStatus = COEStatus.completed;
+    await disbursementScheduleRepo.save(firstDisbursement);
+    // Create approved student appeal


I will add the period.

Period added.

ann-aot · 2023-03-24T19:25:34Z

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

+      appealRequests: [approvedAppealRequest],
+    });
+    approvedAppeal.submittedDate = addDays(-1);
+    // Create declined student appeal from yesterday


I will add the period.

Period added.

...trollers/application/_tests_/application.students.getCompletedApplicationDetails.e2e-spec.ts

sources/packages/web/src/components/students/applicationTracker/ApplicationProgressBar.vue

ann-aot · 2023-03-24T19:56:14Z

sources/packages/web/src/components/students/applicationTracker/Cancelled.vue

@@ -31,7 +31,7 @@ export default defineComponent({
      required: true,
    },
    cancelledDate: {
-      type: String,
+      type: Date,


ann-aot

Good work @andrewsignori-aot . added some comments

dheepak-aot · 2023-03-24T22:36:52Z

sources/packages/backend/apps/api/src/services/application/application.service.ts

@@ -1504,10 +1500,6 @@ export class ApplicationService extends RecordDataModelService<Application> {
        student: {
          id: options?.studentId,
        },
-        studentScholasticStandings: {


andrepestana-aot

Great job! Left a minor comment.

andrepestana-aot · 2023-03-25T00:28:21Z

sources/packages/backend/libs/test-utils/src/factories/student-scholastic-standing.ts

+ * - `submittedBy` institution user submitting the scholastic standing change.
+ * - `application` related student application.
+ * - `studentAssessment` related assessment.
+ * - `student` related student.


Please remove student comment.

dheepak-aot · 2023-03-27T16:15:41Z

sources/packages/web/src/components/students/applicationTracker/ApplicationProgressBar.vue

-  endStatusType?: "success" | "warning" | "error";
-  endStatusIcon?: string;
+class StatusIconDetails {
+  constructor(public statusType?: "success" | "warning" | "error") {}


I see that having the statusType as optional is replicating the pattern of the interface, but I somehow feel like statusType doesn't need to be optional as it would never be expected to be initialized without passing statusType.

This code was refactored, please take a look at the lastest PR updates.

dheepak-aot · 2023-03-27T16:19:06Z

sources/packages/web/src/components/students/applicationTracker/ApplicationProgressBar.vue

-          endStatusType: "error",
-          endStatusIcon: "fa:fas fa-exclamation-circle",
-        };
+        statusIconDetails.value = new StatusIconDetails("success");


I like this way of getting status icon details by associating the dependency between variables. We could start following this patten in such scenarios IMO. 👍

Yes, I thought the same, but after our last Team conversation I thought that it would potentially cause more questions than benefits and that is why I did a refactor here. Please let me know if it is still ok.

github-actions · 2023-03-27T16:22:05Z

Backend Unit Tests Coverage Report

Totals
Statements:	18.01% ( 1949 / 10824 )
Methods:	8.19% ( 115 / 1404 )
Lines:	20.69% ( 1705 / 8240 )
Branches:	10.93% ( 129 / 1180 )

github-actions · 2023-03-27T16:22:59Z

E2E Workflow Workers Coverage Report

Totals
Statements:	31.39% ( 167 / 532 )
Methods:	20.51% ( 16 / 78 )
Lines:	38.3% ( 149 / 389 )
Branches:	3.08% ( 2 / 65 )

github-actions · 2023-03-27T16:24:11Z

E2E Queue Consumers Coverage Report

Totals
Statements:	56.72% ( 308 / 543 )
Methods:	46.38% ( 32 / 69 )
Lines:	59.48% ( 276 / 464 )
Branches:	0% ( 0 / 10 )

dheepak-aot · 2023-03-27T16:24:47Z

sources/packages/web/src/components/students/applicationTracker/ApplicationProgressBar.vue

-          endStatusType: "error",
-          endStatusIcon: "fa:fas fa-exclamation-circle",
-        };
+        statusIconDetails.value = STATUS_ICON_SUCCESS;


We can use the same class concept here and avoid using 3 interfaces.

This is the refactoring of the previous class. There is only one interface, I am not following what would be "avoid using 3 interfaces.".

github-actions · 2023-03-27T16:25:50Z

E2E SIMS API Coverage Report

Totals
Statements:	36.81% ( 2466 / 6699 )
Methods:	28.39% ( 247 / 870 )
Lines:	42.39% ( 2118 / 4997 )
Branches:	12.14% ( 101 / 832 )

dheepak-aot · 2023-03-27T16:30:12Z

Good work @andrewsignori-aot . Thanks for additional refactors. Just one minor comment.

sonarqubecloud · 2023-03-27T16:31:42Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.2% Duplication

dheepak-aot

LGTM. 👍

ann-aot

Good work @andrewsignori-aot 👍 Thanks for doing the changes

guru-aot

LGTM, nice work @andrewsignori-aot

@hellolynn-tbtb

) - Fix for PIR decline throwing an error while completing the process. - Refactor application progress details to get rid of the Sonarcloud issue. - Icon spacing adjusted (@hellolynn-tbtb is aware of and approved it). - Renamed factory method saveFakeApplicationCOE to saveFakeApplicationDisbursements allowing it to be used for these E2E tests also. - Removed prefix ${ClientTypeBaseRoute.Institution} from some E2E that were wrongly added. - E2E tests for the new endpoint added for the completed status. ApplicationStudentsController(e2e)-getCompletedApplicationDetails ✓ Should throw NotFoundException when the application is not associated with the authenticated student. (390 ms) ✓ Should throw NotFoundException when the application is not in 'Completed' status. (75 ms) ✓ Should get application details when application is in 'Completed' status. (76 ms) ✓ Should get application details with scholastic standing change type when application has a scholastic standing 'Student did not complete program associated with. (60 ms) ✓ Should get application details with the most updated appeal status when the application has more than one student appeals associated with. (78 ms)

@hellolynn-tbtb

) - Fix for PIR decline throwing an error while completing the process. - Refactor application progress details to get rid of the Sonarcloud issue. - Icon spacing adjusted (@hellolynn-tbtb is aware of and approved it). - Renamed factory method saveFakeApplicationCOE to saveFakeApplicationDisbursements allowing it to be used for these E2E tests also. - Removed prefix ${ClientTypeBaseRoute.Institution} from some E2E that were wrongly added. - E2E tests for the new endpoint added for the completed status. ApplicationStudentsController(e2e)-getCompletedApplicationDetails ✓ Should throw NotFoundException when the application is not associated with the authenticated student. (390 ms) ✓ Should throw NotFoundException when the application is not in 'Completed' status. (75 ms) ✓ Should get application details when application is in 'Completed' status. (76 ms) ✓ Should get application details with scholastic standing change type when application has a scholastic standing 'Student did not complete program associated with. (60 ms) ✓ Should get application details with the most updated appeal status when the application has more than one student appeals associated with. (78 ms)

First E2E tests for getCompletedApplicationDetails

e548084

andrewsignori-aot self-assigned this Mar 23, 2023

andrewsignori-aot added SIMS-Api SIMS-Api E2E/Unit tests labels Mar 23, 2023

andrewsignori-aot added 2 commits March 22, 2023 20:03

Sonarcloud fixes

955e60f

Progress bar adjustments

ec313af

andrewsignori-aot commented Mar 23, 2023

View reviewed changes

sources/packages/backend/apps/api/src/services/application/application.service.ts Show resolved Hide resolved

Comments improvements

bc25e6b

andrewsignori-aot marked this pull request as ready for review March 24, 2023 00:03

andrewsignori-aot requested review from dheepak-aot, ann-aot, guru-aot, sh16011993, hellolynn-tbtb, JasonCTang and andrepestana-aot March 24, 2023 00:03

Made statusType optional

f7745a0