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.

Sign up for GitHub

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

Jump to bottom

#1852 - Enable Overawards Tab For Public Institution User #1906

Merged
merged 16 commits into from
May 2, 2023
Merged

#1852 - Enable Overawards Tab For Public Institution User #1906

merged 16 commits into from
May 2, 2023

Conversation

sh16011993
Copy link
Collaborator

@sh16011993 sh16011993 commented Apr 26, 2023 •
edited
Loading

The following tasks were completed as a part of this task:

  • "Add Manual Record" button must not be present for institutions
  • Wrote E2E tests: This includes writing the following e2e tests:
    1. Ensure correct value for overaward balance is returned
    2. Student overawards are returned as expected

Screenshots showing the Overawards tab in the Institution Portal:

image

image

@sh16011993 sh16011993 added Content Tasks related to UIUX content Web Portal SIMS-Api SIMS-Api labels Apr 26, 2023
@sh16011993 sh16011993 self-assigned this Apr 26, 2023
andrewsignori-aot
andrewsignori-aot reviewed Apr 26, 2023
View reviewed changes
...rs/overaward/_tests_/e2e/overaward.institution.controller.getOverawardsByStudent.e2e-spec.ts Outdated

it("Should return student overawards", async () => {
// Arrange
const user = await userRepo.save(createFakeUser());
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible to simplify the "Arrange" area using the saveFakeApplication?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless I am missing something, student and user will also be created by saveFakeApplication so we would not need the 49 and 50.

andrewsignori-aot
andrewsignori-aot reviewed Apr 26, 2023
View reviewed changes
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.

Nice job, please take a look at the comments.

ann-aot
ann-aot reviewed Apr 26, 2023
View reviewed changes
Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

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

Good work @sh16011993 . Added some comments

ann-aot
ann-aot reviewed Apr 26, 2023
View reviewed changes
sources/packages/backend/package.json Outdated
@@ -22,7 +22,7 @@
"test:watch": "jest --watch ",
"test:cov": "cross-env ENVIRONMENT=test jest --coverage --forceExit",
"test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
"test:e2e:api": "npm run db:seed:test filter=CreateInstitutionsAndAuthenticationUsers,CreateAESTUsers,CreateStudentUsers && cross-env ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/api/test/jest-e2e.json --forceExit",
"test:e2e:api": "npm run db:seed:test filter=CreateInstitutionsAndAuthenticationUsers,CreateAESTUsers,CreateStudentUsers && cross-env NODE_OPTIONS=--max_old_space_size=8192 ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/api/test/jest-e2e.json --forceExit",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ann-aot ann-aot mentioned this pull request Apr 26, 2023
Merged
andrepestana-aot
andrepestana-aot reviewed Apr 26, 2023
View reviewed changes
...llers/overaward/_tests_/e2e/overaward.institution.controller.getOverawardBalance.e2e-spec.ts Outdated
disbursementOverawardRepo = dataSource.getRepository(DisbursementOveraward);
});

it("Should return correct value for overaward balance", async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should create the test description with Should/when. e.g.: "Should return correct value for overaward balance when
student has some overawards"

@dheepak-aot
Copy link
Collaborator

Thanks for making the changes @sh16011993. One last comment on files names.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

github-actions bot commented May 1, 2023

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 18.02% ( 1993 / 11062 )
Methods: 8.1% ( 116 / 1432 )
Lines: 20.82% ( 1750 / 8405 )
Branches: 10.37% ( 127 / 1225 )

@github-actions
Copy link

github-actions bot commented May 1, 2023

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 32.41% ( 176 / 543 )
Methods: 21.25% ( 17 / 80 )
Lines: 39.25% ( 157 / 400 )
Branches: 3.17% ( 2 / 63 )

@github-actions
Copy link

github-actions bot commented May 1, 2023

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 66.67% ( 362 / 543 )
Methods: 55.07% ( 38 / 69 )
Lines: 68.97% ( 320 / 464 )
Branches: 40% ( 4 / 10 )

@github-actions
Copy link

github-actions bot commented May 1, 2023

E2E SIMS API Coverage Report

Totals Coverage
Statements: 39.96% ( 2729 / 6829 )
Methods: 32.96% ( 293 / 889 )
Lines: 45.26% ( 2305 / 5093 )
Branches: 15.47% ( 131 / 847 )

dheepak-aot
dheepak-aot approved these changes May 1, 2023
View reviewed changes
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.

Thanks for making the changes @sh16011993 . LGTM.

andrepestana-aot
andrepestana-aot approved these changes May 1, 2023
View reviewed changes
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.

LGTM!

guru-aot
guru-aot approved these changes May 1, 2023
View reviewed changes
Copy link
Collaborator

@guru-aot guru-aot left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @sh16011993

ann-aot
ann-aot approved these changes May 1, 2023
View reviewed changes
Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

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

Good work @sh16011993 👍 Thanks for doing the changes 👍

andrewsignori-aot
andrewsignori-aot approved these changes May 2, 2023
View reviewed changes
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 doing the changes, looks good 👍

@sh16011993 sh16011993 merged commit b0788f5 into main May 2, 2023
@sh16011993 sh16011993 temporarily deployed to DEV May 2, 2023 18:05 — with GitHub Actions Inactive
@sh16011993 sh16011993 deleted the 1852_enable_overawards_tab_public_institution_user branch May 2, 2023 18:05
@sh16011993 sh16011993 temporarily deployed to DEV May 2, 2023 18:05 — with GitHub Actions Inactive
@sh16011993 sh16011993 temporarily deployed to DEV May 2, 2023 18:16 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrepestana-aot andrepestana-aot andrepestana-aot approved these changes

@ann-aot ann-aot ann-aot approved these changes

@guru-aot guru-aot guru-aot approved these changes

@andrewsignori-aot andrewsignori-aot andrewsignori-aot approved these changes

@dheepak-aot dheepak-aot dheepak-aot approved these changes

@hellolynn-tbtb hellolynn-tbtb Awaiting requested review from hellolynn-tbtb

@JasonCTang JasonCTang Awaiting requested review from JasonCTang

Assignees

@sh16011993 sh16011993

Labels
Content Tasks related to UIUX content E2E/Unit tests SIMS-Api SIMS-Api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sh16011993 @dheepak-aot @andrewsignori-aot @guru-aot @ann-aot @andrepestana-aot @JasonCTang