Skip to content

#3553 - Revise Impact of BC Lifetime Max Restriction to only block Loan#5592

Merged
andrewsignori-aot merged 15 commits intomainfrom
feature/#3553-revise-impact-of-bc-lifetime
Jan 10, 2026
Merged

#3553 - Revise Impact of BC Lifetime Max Restriction to only block Loan#5592
andrewsignori-aot merged 15 commits intomainfrom
feature/#3553-revise-impact-of-bc-lifetime

Conversation

@andrewsignori-aot
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot commented Jan 8, 2026

  • Split the StopFullTimeBCFunding into StopFullTimeBCLoan and StopFullTimeBCGrants to allow restriction to be configured with a more granular option.
  • Renamed the StopPartTimeBCFunding to StopPartTimeBCGrants since part-time has no loan.
  • Update restrictions B2D, B3D, B2, Z1 to use the new action types, keeping their existing behavior.
  • Update restrictions BCLM to block only BC Loan while still allowing grants disbursements.
  • Restrictions bypass conditions to list restrictions were adapted to include the new restriction action types.

E2E Tests

  • Existing BCLM tests adapted to validate if grants are still disbursed.
  • Created a new test to ensure different restriction IDs are correctly associated with blocked awards.
  • Some E2E tests were selecting an institution restriction when the context required a student restriction due to the lack of some where conditions. Restriction provincial type was used for those scanrios.

Migration Rollback

image

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors BC funding restrictions to provide more granular control by separating restrictions into distinct loan and grant types. Previously, StopFullTimeBCFunding and StopPartTimeBCFunding restricted all BC funding together. Now, full-time restrictions are split into StopFullTimeBCLoan (BCSL) and StopFullTimeBCGrants (BCAG, BGPD, SBSD), while StopPartTimeBCFunding is renamed to StopPartTimeBCGrants (BCAG, SBSD). The BCLM restriction is updated to block only BC loans while allowing grants to be disbursed.

Key changes:

  • Database migration adds new restriction action types and updates existing restrictions to use them
  • E-Cert generation logic now evaluates restrictions separately for loan vs. grant funding types
  • Comprehensive test coverage validates the new behavior, including scenarios with multiple restrictions

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
RestrictionContract.ts (web) Updated frontend enum with new restriction action types and consistent naming (full-time/part-time with hyphens)
restriction-action-type.type.ts Updated backend enum to match frontend, splitting BC funding into loan and grant types
e-cert-steps-utils.ts Added logic to map restrictions to specific funding types (loan vs grant) and build restriction map per disbursement value type
apply-stop-bc-funding-restriction-step.ts Refactored to use new funding type restriction map instead of single restriction check
apply-overawards-balance-step.ts Updated to use new funding type map for determining which awards should not be deducted from overawards
validate-disbursement-base.ts Updated function signature to accept array of action types
application-restriction-bypass.service.ts Updated queries to check for both loan and grant restriction types
application-restriction-bypass.ts (test factory) Added RestrictionType filter to ensure proper restriction selection in tests
Update-restriction-action-types-BC-funding-to-loan-and-grants.sql Migration to create new enum with split types and update column values
Rollback-update-restriction-action-types-BC-funding-to-loan-and-grants.sql Rollback migration to revert enum types (incomplete for BCLM)
Update-stop-funding-loan-and-grants.sql Updates specific restrictions (BCLM, B2D, B3D, B2, Z1, B6A) to use new action types
Rollback-update-stop-funding-loan-and-grants.sql Reverts restrictions to previous types (incomplete for BCLM)
UpdateBCFundingToLoanAndGrants1767817093895.ts Migration class that executes SQL migrations in correct order
ecert-full-time-process-integration.scheduler.e2e-spec.ts Added test for multiple restrictions blocking both loan and grant; updated existing tests for new BCLM behavior
ecert-part-time-process-integration.scheduler.e2e-spec.ts Updated tests to use new restriction types and verify proper filtering
e-cert-utils.ts Enhanced awardAssert to support validation of specific restriction IDs
disbursement-schedule.models.ts Removed unused interfaces

@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review January 9, 2026 20:01
@dheepak-aot dheepak-aot self-requested a review January 9, 2026 20:25
@sh16011993 sh16011993 self-requested a review January 9, 2026 21:04
Comment on lines +201 to 205
RestrictionActionType.StopFullTimeBCLoan,
RestrictionActionType.StopFullTimeBCGrants,
RestrictionActionType.StopFullTimeDisbursement,
RestrictionActionType.StopPartTimeBCFunding,
RestrictionActionType.StopPartTimeBCGrants,
RestrictionActionType.StopPartTimeDisbursement,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +963 to +964
restrictionAmountSubtracted: 0,
effectiveAmount: 499,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +1891 to +1892
restrictionType: RestrictionType.Provincial,
actionEffectiveConditions: IsNull(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines 1418 to 1427
const restriction = await db.restriction.findOne({
select: { id: true },
where: {
restrictionType: RestrictionType.Provincial,
actionEffectiveConditions: IsNull(),
actionType: ArrayContains([
RestrictionActionType.StopPartTimeBCFunding,
RestrictionActionType.StopPartTimeBCGrants,
]),
},
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we are seeing this very often, it seems like a good candidate to be in factory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, but this PR is not even using new "copies" of it, and the highlighted changes are already some minor fixes and improvements. I will refrain from doing it at this moment.

/**
* Map of restriction action types associated with the offering intensity.
*/
const RESTRICTION_ACTION_TYPE_INTENSITY_MAP = new Map([
Copy link
Collaborator

Choose a reason for hiding this comment

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

STOP_FUNDING_ACTION_INTENSITY_MAP ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 9, 2026

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 20.29% ( 4324 / 21313 )
Methods: 9.65% ( 252 / 2611 )
Lines: 24.44% ( 3706 / 15164 )
Branches: 10.34% ( 366 / 3538 )

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 75.41% ( 1055 / 1399 )
Methods: 79.31% ( 115 / 145 )
Lines: 78.79% ( 769 / 976 )
Branches: 61.51% ( 171 / 278 )

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 85.68% ( 1616 / 1886 )
Methods: 85% ( 187 / 220 )
Lines: 88.64% ( 1287 / 1452 )
Branches: 66.36% ( 142 / 214 )

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

E2E SIMS API Coverage Report

Totals Coverage
Statements: 75.57% ( 8582 / 11357 )
Methods: 75.59% ( 1028 / 1360 )
Lines: 79.64% ( 6235 / 7829 )
Branches: 60.84% ( 1319 / 2168 )

if (!restriction) {
log.info("Checking stop funding restriction.");
const stopFundingMap =
getStopFundingTypesAndRestrictionsMap(eCertDisbursement);
Copy link
Collaborator

@dheepak-aot dheepak-aot Jan 9, 2026

Choose a reason for hiding this comment

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

I feel that the existing idea of using the shouldStopBCFunding could be more simple and used with minor enhancement.
With a draft idea like this.

export function getStopFundingRestriction(
  eCertDisbursement: EligibleECertDisbursement,
  disbursementValue: DisbursementValue,
): ActiveRestriction | undefined {
  const stopFullTimeFundingActionTypeMap = {
    [DisbursementValueType.BCGrant]: RestrictionActionType.StopFullTimeBCGrants,
    [DisbursementValueType.BCTotalGrant]:
      RestrictionActionType.StopFullTimeBCGrants,
    [DisbursementValueType.BCLoan]: RestrictionActionType.StopFullTimeBCLoan,
  };
  const stopPartTimeFundingActionTypeMap = {
    [DisbursementValueType.BCGrant]: RestrictionActionType.StopPartTimeBCGrants,
    [DisbursementValueType.BCTotalGrant]:
      RestrictionActionType.StopPartTimeBCGrants,
  };
  const stopFundingActionTypeMap =
    eCertDisbursement.offering.offeringIntensity === OfferingIntensity.fullTime
      ? stopFullTimeFundingActionTypeMap
      : stopPartTimeFundingActionTypeMap;
  const restrictionActionType =
    stopFundingActionTypeMap[disbursementValue.valueType];
  if (!restrictionActionType) {
    return;
  }
  const [stopFundingRestriction] = getRestrictionsByActionType(
    eCertDisbursement,
    restrictionActionType,
  );
  return stopFundingRestriction;
}
``

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • The proposed method does not consider the fact that multiple restrictions can possibly impact an award. The idea was to create a method with the ability to expose all the restrictions, supporting the current approach (that requires only one) but also future implementations.
  • The BCTotalGrant is purposely no longer considered as it should result as zero due to the other grants.
  • I am missing what would be the "minor enhancement" with the proposed solution.
    Please let me know if it would be a blocker.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The outcome of the method put in the PR is good. Only feeling was that the method handles data at multiple levels which makes it look confusing. But not a blocker.

Copy link
Collaborator

@dheepak-aot dheepak-aot left a comment

Choose a reason for hiding this comment

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

Great work and improvement to the granularity of the funding type restrictions. 👍

Comment on lines +127 to +140
resultMap.set(DisbursementValueType.BCLoan, [
...(resultMap.get(DisbursementValueType.BCLoan) || []),
restriction,
]);
}
if (
BC_GRANTS_RESTRICTION_ACTIONS.some((action) =>
restriction.actions.includes(action),
)
) {
resultMap.set(DisbursementValueType.BCGrant, [
...(resultMap.get(DisbursementValueType.BCGrant) || []),
restriction,
]);
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.

Splendid work @andrewsignori-aot 👍 Thanks for clarifying my questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants