-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3965 - Separate Sequencing for FT and PT eCert Numbers #4207
Conversation
.../backend/apps/db-migrations/src/migrations/1736193026064-RenameSequenceNameDocumentNumber.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/apps/db-migrations/src/sql/SequenceControl/Rename-sequence-name.sql
Outdated
Show resolved
Hide resolved
...s/backend/libs/services/src/confirmation-of-enrollment/confirmation-of-enrollment.service.ts
Show resolved
Hide resolved
); | ||
}); |
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 think we can also assert that the sequence control record created by the COE operation has the correct sequence name, which is "Part Time_DISBURSEMENT_DOCUMENT_NUMBER", and the sequence number is the document number for the disbursement schedule record of the application.
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.
Added the sequence Control Assert test.
sources/packages/backend/apps/db-migrations/src/sql/SequenceControl/Rename-sequence-name.sql
Outdated
Show resolved
Hide resolved
sources/packages/backend/apps/db-migrations/src/sql/SequenceControl/Rename-sequence-name.sql
Outdated
Show resolved
Hide resolved
|
||
it("Should allow the COE confirmation when the application is on Enrolment status and all the conditions are fulfilled for Part Time offering.", async () => { | ||
// Arrange |
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.
Not a blocker. As a recommendation, since the PR is to separate FT and PT sequence names, it would be nice to include a test case for FT application to verify the sequence name as "Full Time_DISBURSEMENT_DOCUMENT_NUMBER".
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.
Added the sequence Control Assert test for Full Time.
Nice work for getting the gist of the story in this PR 👍 Please take a look at some minor comments. |
...s/backend/libs/services/src/confirmation-of-enrollment/confirmation-of-enrollment.service.ts
Show resolved
Hide resolved
...enrollment/_tests_/e2e/confirmation-of-enrollment.institutions.confirmEnrollment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
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.
Nice work, please take a look at the comments.
...enrollment/_tests_/e2e/confirmation-of-enrollment.institutions.confirmEnrollment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...enrollment/_tests_/e2e/confirmation-of-enrollment.institutions.confirmEnrollment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
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.
Thanks for making the changes, only minor changes left.
Quality Gate passedIssues Measures |
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.
Thanks for making the changes, nice work, looks good 👍
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.
PR looks great. Nice work 👍
Technical
DISBURSEMENT_DOCUMENT_NUMBER
toPart Time_DISBURSEMENT_DOCUMENT_NUMBER
.Db Migrations: Run
Db Migrations:: Demo
Db Migrations: Revert
Up Insert: non-PROD (DEV/TEST/Staging).
Extra E2E Test
sources/packages/backend/apps/api/src/route-controllers/confirmation-of-enrollment/_tests_/e2e/confirmation-of-enrollment.institutions.confirmEnrollment.e2e-spec.ts