Skip to content
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

#4015 - FT program year total contribution limits #4154

Merged
merged 27 commits into from
Dec 23, 2024

Conversation

guru-aot
Copy link
Collaborator

@guru-aot guru-aot commented Dec 19, 2024

  • AssessmentGateway workflow has been changed to save the following data in the workflow data
  • calculatedDataExemptScholarshipsBursaries
  • calculatedDataSpouseContributionWeeks
  • calculatedDataTotalFederalFSC
  • calculatedDataTotalProvincialFSC

image

  • Refactored the method getProgramYearPreviousAwardsTotals to reduce the call to the database in fetching the sequenced application and assessment.
  • Created a private method as requested in the ticket and additionally created another method getFTProgramYearContributionTotals to process the contribution total logic.
  • E2E test case was updated to show the created variables and new E2E created to test the sum logic in the query.
  • programYearTotalScholarshipsBursaries
  • programYearTotalSpouseContributionWeeks
  • programYearTotalFederalFSC
  • programYearTotalProvincialFSC

Note: PR separation is not done, as the workflow change was very minor and it can be accommodated in a single PR.

@guru-aot guru-aot changed the title Feature/#4015 ft program year limits #4015 - FT program year Contribution limits Dec 20, 2024
@guru-aot guru-aot changed the title #4015 - FT program year Contribution limits #4015 - FT program year total contribution limits Dec 20, 2024
@guru-aot guru-aot self-assigned this Dec 20, 2024
@guru-aot guru-aot added Camunda Workers Camunda Worflow Involves camunda workflow changes labels Dec 20, 2024
@guru-aot guru-aot marked this pull request as ready for review December 20, 2024 17:30
Comment on lines +977 to +980
programYearTotalFederalFSC: 0,
programYearTotalProvincialFSC: 0,
programYearTotalScholarshipsBursaries: 0,
programYearTotalSpouseContributionWeeks: 0,
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot Dec 20, 2024

Choose a reason for hiding this comment

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

Same question for a later comment, should we return 0 when values are not defined?
If the SUM results in 0 I do not see a problem, but having 0 as a fallback for an undefined value, do we need that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we are not doing any calculation still in the workflow, i thought it would be better to return 0 than null, as the variable name states "programYearTotal" either it can be 0 or some numeric value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My point is the variable programYearTotal* will not be even there if not present.

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot left a 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, please take a look at the latest comments.

!!applicationNumbers?.length;
const [awardTotals, contributionTotals] = await Promise.all([
// Get the program year awards totals for part-time and full-time.
await this.getProgramYearPreviousAwardsTotals(
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need to await here.

/**
* Full-time program year contribution and its totals.
*/
export interface programYearContributionTotal {
Copy link
Collaborator

@andrepestana-aot andrepestana-aot Dec 20, 2024

Choose a reason for hiding this comment

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

programYearContributionTotal should start with a capital letter.

Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 22.08% ( 3785 / 17140 )
Methods: 10.09% ( 219 / 2171 )
Lines: 25.49% ( 3280 / 12870 )
Branches: 13.63% ( 286 / 2099 )

Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 65.59% ( 589 / 898 )
Methods: 59.63% ( 65 / 109 )
Lines: 68.72% ( 468 / 681 )
Branches: 51.85% ( 56 / 108 )

Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 82.7% ( 1291 / 1561 )
Methods: 70.05% ( 131 / 187 )
Lines: 85.67% ( 1082 / 1263 )
Branches: 70.27% ( 78 / 111 )

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot left a 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 👍

Copy link

E2E SIMS API Coverage Report

Totals Coverage
Statements: 67.4% ( 5922 / 8787 )
Methods: 65.09% ( 729 / 1120 )
Lines: 71.33% ( 4647 / 6515 )
Branches: 47.4% ( 546 / 1152 )

Copy link
Collaborator

@andrepestana-aot andrepestana-aot left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the changes!

@guru-aot guru-aot added this pull request to the merge queue Dec 23, 2024
Merged via the queue into main with commit ca835ac Dec 23, 2024
21 checks passed
@guru-aot guru-aot deleted the feature/#4015-FT-ProgramYearLimits branch December 23, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Camunda Worflow Involves camunda workflow changes Camunda Workers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants