#3453 - COE for $0/Not Eligible Assessments - Part 2

[lewischen-aot]

This PR resolves the following business AC:

• Do not show $0/Ineligible requests in the COE Requests Report.
• Do not send ECE requests to institutions for $0/Ineligible COE requests.

This PR covers the changes for COE Request report and e-Cert integration/queue consumer.

• Updated COE Request report SQL script to exclude disbursements that don't have estimated awards.
• Used the flag for the e-Cert validations, integration and queueu-consumers.
• Modified existing E2E tests for ECE and API
  • Added test case "Should generate the COE Requests report without application(s) that don't have estimated awards when one or more applications exist for the given institution."

Rollback Evidence

[dheepak-aot]

Minor. Please format the sql with our VS code formatting. Same for the revert.

It can be done by copying query to new text file and doing this.

[dheepak-aot]

Please let me know if you need help with the formatting.

[dheepak-aot]

Great Job @lewischen-aot . Please have a look at the comments. Most of them are minor comments.

Also please add a E2E test for ECE as per the technical AC.

[dheepak-aot]

Please check the comment. It seems to be moved over from e-certs.

[dheepak-aot]

Can we have the COE Status deliberately mentioned?

[dheepak-aot]

I would recommend the file detail to assert exact match instead of contain.

I can be achieved by creating a parser.

function buildECEFileDetail(
      application: Application,
      disbursementValue: DisbursementValue,
      disbursementDate: string,
    ): string {
      const DATE_FORMAT = "YYYYMMDD";
      const offering = application.currentAssessment.offering;
      const studyStartDate = formatDate(offering.studyStartDate, DATE_FORMAT);
      const studyEndDate = formatDate(offering.studyEndDate, DATE_FORMAT);
      const disbursementDateFormatted = formatDate(
        disbursementDate,
        DATE_FORMAT,
      );
      const institutionLocation = offering.institutionLocation;
      const student = application.student;
      const studentFirstName = student.user.firstName?.substring(0, 15) ?? "";
      const studentLastName = student.user.lastName.substring(0, 25);
      const studentBirthDate = formatDate(student.birthDate, DATE_FORMAT);
      const institutionStudentNumber = application.studentNumber;
      return `${RecordTypeCodes.ECEDetail}${
        institutionLocation.institutionCode
      }${disbursementValue.id.toString().padStart(10, "0")}${
        disbursementValue.valueCode
      }${disbursementValue.valueAmount.toString().padStart(9, "0")}${
        student.sinValidation.sin
      }${studentLastName.padEnd(25, " ")}${studentFirstName.padEnd(
        15,
        " ",
      )}${studentBirthDate}${
        application.applicationNumber
      }${institutionStudentNumber.padEnd(
        12,
        " ",
      )}100${studyStartDate}${studyEndDate}${disbursementDateFormatted}`;
    }

and calling the parser like this.

const [disbursement] =
        application.currentAssessment.disbursementSchedules;
      const [disbursementValue] = disbursement.disbursementValues;
      const expectedDetailRecord = buildECEFileDetail(
        application,
        disbursementValue,
        disbursement.disbursementDate,
      );
      expect(fileDetail).toBe(expectedDetailRecord);

[dheepak-aot]

I also tested the result before sharing the above suggestion.

[dheepak-aot]

Please assert the footer like this.
expect(footer).toBe("3000001");

[dheepak-aot]

Please use the util describeQueueProcessorRootTest(QueueNames.ECEProcessIntegration) to follow other scheduler descriptions

[dheepak-aot]

Comments still say e-cert. Please adjust the comments.

[dheepak-aot]

Thanks for doing the changes. Looks good 👍

Please address the 2 minor comments before merge.

#4027 (comment)
#4027 (comment)

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Backend Unit Tests Coverage Report

Totals
Statements:	21.97% ( 3753 / 17083 )
Methods:	10.1% ( 215 / 2129 )
Lines:	25.32% ( 3258 / 12869 )
Branches:	13.43% ( 280 / 2085 )

[github-actions]

E2E Workflow Workers Coverage Report

Totals
Statements:	65.43% ( 583 / 891 )
Methods:	59.26% ( 64 / 108 )
Lines:	68.54% ( 464 / 677 )
Branches:	51.89% ( 55 / 106 )

[github-actions]

E2E Queue Consumers Coverage Report

Totals
Statements:	84.94% ( 1342 / 1580 )
Methods:	83.33% ( 135 / 162 )
Lines:	86.45% ( 1129 / 1306 )
Branches:	69.64% ( 78 / 112 )

[github-actions]

E2E SIMS API Coverage Report

Totals
Statements:	67.16% ( 5869 / 8739 )
Methods:	64.75% ( 722 / 1115 )
Lines:	71.15% ( 4612 / 6482 )
Branches:	46.85% ( 535 / 1142 )

[andrepestana-aot]

Why do we need the studentNumber? That is nullable in sims.applications.

[andrepestana-aot]

Please change buildECEFileDetail to do not require application.studentNumber as it's not required for the e-Cert generation. (Edit: I meant ECE)

[lewischen-aot]

Good point @andrepestana-aot. I also thought that student number was not required, until the E2E test failed due to the null value from institutionStudentNumber used by the following code in the method getFixedFormat() in ece-file-detail.ts:

      record.appendWithEndFiller(
        this.institutionStudentNumber,
        12,
        SPACE_FILLER,
      );

In this case, institutionStudentNumber is studentNumber obtained from another method. When the value institutionStudentNumber is null, the method appendWithEndFiller will raise a type error as shown below.

Setting a value for studentNumber for the application in the E2E test will make it pass. In short, student number should be a mandatory field for submitting student applications, and the ECE request process integration ensures that student number is required. If there is any further doubts about this, I think we can raise the question in DEV meetings and make corresponding changes in separate story.

[andrepestana-aot]

Ok, I see. That is required for the ECE integration. Thanks for checking. We can confirm that later with other DEVS.

[andrepestana-aot]

Good job! Just a minor comment.

[andrepestana-aot]

Thanks for checking my question. Looks good!