-
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
#3710 - Stop part time BC funding - Implement new restriction effect #3956
#3710 - Stop part time BC funding - Implement new restriction effect #3956
Conversation
763a558
to
04257ba
Compare
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Outdated
Show resolved
Hide resolved
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Outdated
Show resolved
Hide resolved
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Outdated
Show resolved
Hide resolved
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Outdated
Show resolved
Hide resolved
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Outdated
Show resolved
Hide resolved
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.ts
Show resolved
Hide resolved
const b6aRestriction = await db.restriction.findOneBy({ | ||
restrictionCode: RestrictionCode.B6A, | ||
}); |
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.
No need to be specific with the restriction if we find it as below (code from the same file).
No a blocker but it would prevent dealing with the restriction code directly since the E2E test is about the StopPartTimeBCFunding
and not about a specific restriction.
const restriction = await db.restriction.findOne({
where: {
actionType: ArrayContains([
RestrictionActionType.StopPartTimeBCFunding,
]),
},
});
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, but the above code is used in the test case
Line 1057 in 3eec04e
it( |
and can be used at beforeAll part of the test case and passed in both the implementations.
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.
Thank you suggestion,
Updated the E2E to get restriction instead of B6A.
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 change.
@guru-aot I am approving the PR since this entire comment was just a suggestion even though I saw value in your suggested change.
...egration/ecert-integration/_tests_/ecert-part-time-process-integration.scheduler.e2e-spec.ts
Show resolved
Hide resolved
...egration/ecert-integration/_tests_/ecert-part-time-process-integration.scheduler.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.
...ices/disbursement-schedule/e-cert-processing-steps/apply-stop-bc-funding-restriction-step.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 @bidyashish , please have a look on the comments.
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, 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.
LGTM nice work @bidyashish
Technical